Skip to content

Convection diagnostics needed for PS49 - #676

Open
iboutle wants to merge 5 commits into
MetOffice:mainfrom
iboutle:conv_icao_cape_diags
Open

iboutle wants to merge 5 commits into
MetOffice:mainfrom
iboutle:conv_icao_cape_diags

Conversation

@iboutle

@iboutle iboutle commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

PR Summary

Sci/Tech Reviewer: MichaelWhitall
Code Reviewer: Ed Hone (@EdHone)

UM stashcode LFRic ID
5-224 lowest_conv_cloud_base_icao_height
5-225 lowest_conv_cloud_top_icao_height
5-233 cape_undilute
5-213 ccw_unscaled

Some plots of the new diagnostics:
Screenshot from 2026-07-30 09-17-41
Screenshot from 2026-07-30 09-17-19
Screenshot from 2026-07-29 16-27-32
Screenshot from 2026-08-10 15-35-01

closes #643

Code Quality Checklist

  • I have performed a self-review of my own code
  • My code follows the project's style guidelines
  • Comments have been included that aid understanding and enhance the readability of the code
  • My changes generate no new warnings
  • All automated checks in the CI pipeline have completed successfully

Testing

  • I have tested this change locally, using the LFRic Apps rose-stem suite
  • If any tests fail (rose-stem or CI) the reason is understood and acceptable (e.g. kgo changes)
  • I have added tests to cover new functionality as appropriate (e.g. system tests, unit tests, etc.)
  • Any new tests have been assigned an appropriate amount of compute resource and have been allocated to an appropriate testing group (i.e. the developer tests are for jobs which use a small amount of compute resource and complete in a matter of minutes)

trac.log

Test Suite Results - lfric_apps - conv_icao_cape_diags/run1

Suite Information

Item Value
Suite Name conv_icao_cape_diags/run1
Suite User ian.boutle
Workflow Start 2026-07-29T15:29:43
Groups Run developer', 'lfric_atm_nwp_gal9_oper-C224_MG_ex1a_cce_production-32bit
Dependency Reference Main Like
casim MetOffice/casim@2026.07.1 True
jules MetOffice/jules@2026.07.1 True
lfric_apps iboutle/lfric_apps@conv_icao_cape_diags False
lfric_core MetOffice/lfric_core@2026.07.1 True
moci MetOffice/moci@2026.07.1 True
SimSys_Scripts MetOffice/SimSys_Scripts@2026.07.1 True
socrates MetOffice/socrates@2026.07.1 True
socrates-spectral MetOffice/socrates-spectral@2026.07.1 True
ukca MetOffice/ukca@2026.07.1 True

Task Information

✅ succeeded tasks - 1221

Security Considerations

  • I have reviewed my changes for potential security issues
  • Sensitive data is properly handled (if applicable)
  • Authentication and authorisation are properly implemented (if applicable)

Performance Impact

  • Performance of the code has been considered and, if applicable, suitable performance measurements have been conducted

AI Assistance and Attribution

  • Some of the content of this change has been produced with the assistance of Generative AI tool name (e.g., Met Office Github Copilot Enterprise, Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the Simulation Systems AI policy (including attribution labels)

Documentation

  • Where appropriate I have updated documentation related to this change and confirmed that it builds correctly

PSyclone Approval

  • If you have edited any PSyclone-related code (e.g. PSyKAl-lite, Kernel interface, optimisation scripts, LFRic data structure code) then please contact the TCD Team

Sci/Tech Review

  • I understand this area of code and the changes being added
  • The proposed changes correspond to the pull request description
  • Documentation is sufficient (do documentation papers need updating)
  • Sufficient testing has been completed

(Please alert the code reviewer via a tag when you have approved the SR)

Code Review

  • All dependencies have been resolved
  • Related Issues have been properly linked and addressed
  • CLA compliance has been confirmed
  • Code quality standards have been met
  • Tests are adequate and have passed
  • Documentation is complete and accurate
  • Security considerations have been addressed
  • Performance impact is acceptable

Ubuntu and others added 3 commits July 29, 2026 11:05
Adds two single level diagnostics giving the ICAO standard atmosphere
height, in kft, of the base and the top of the lowest convective cloud
in the column:

  convection__lowest_conv_cloud_base_icao_height
  convection__lowest_conv_cloud_top_icao_height

These are the "lowest cloud" counterparts of the existing
convection__conv_cloud_base_icao_height and
convection__conv_cloud_top_icao_height diagnostics, and are computed in
exactly the same way: icao_heights_kernel is applied to the existing
convection__pres_lowest_cv_base and convection__pres_lowest_cv_top
pressure diagnostics, which are activated as dependencies when either
of the new heights is requested.  Those pressures are already set to
the missing data indicator in columns with no convective cloud, which
icao_heights_kernel passes straight through, so unconverged columns are
flagged as missing rather than given a spurious height.

Both the Gregory-Rowntree and the CoMorph diagnostics paths are
updated, and the new fields are added to the three hourly standard
level output stream alongside the existing ICAO heights.

Some of the content of this change has been produced with the
assistance of Anthropic Claude Opus 5 (Claude Code).
Adds a single level diagnostic, convection__cape_undilute, giving the
convective available potential energy of an undilute parcel ascent in
J/kg.

The quantity is already computed by conv_diag, which in LFRic is called
from bl_exp_kernel rather than from the convection scheme, so the
diagnostic is plumbed out of the explicit boundary layer kernel
following the pattern of the existing zht and oblen diagnostics: a new
write-only single level field is added to the kernel metadata and is
only filled when the field has been requested.

The field is added to the three hourly standard level output stream,
and to the coupled atmosphere field definitions alongside the existing
diluted CAPE.

Some of the content of this change has been produced with the
assistance of Anthropic Claude Opus 5 (Claude Code).
@github-actions github-actions Bot added the cla-modified The CLA has been modified as part of this PR - added by GA label Jul 29, 2026
@iboutle iboutle added this to the Autumn 2026 milestone Jul 29, 2026
@iboutle
iboutle marked this pull request as ready for review July 30, 2026 09:18
@github-actions
github-actions Bot requested a review from MichaelWhitall July 30, 2026 09:19

@MichaelWhitall MichaelWhitall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi thanks for this;

I'm slightly nervous that this change has been written by AI and sci/tech reviewed by someone (me) who doesn't really understand the LFRic diagnostic system in-which the changes are made? But I've had a go, hopefully code-owners who are more in-the-know can check this over too?

Cheers!
Mike

"parcel_buoyancy", "qsat_at_lcl", "bl_type_ind", "visc_m_blend",
"visc_h_blend", "zh_2d", "zhsc_2d", "ntml_2d", "cumulus_2d",
"rh_crit", "mix_len_bm", "dsldzm", "wvar", "zht", "oblen",
"cape_undilute_2d",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what this is doing? Why are we telling PsyClone to "ignore dependencies" specifically for the undilute cape diagnostic output by conv_diag?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure - I'm hoping Hacka Fett (@christophermaynard) is going to say whether this is correct or not (it is at least consistent with what happens for the other diagnostic fields!)

Comment thread rose-stem/app/lfric_atm/file/file_def_diags_oper_nwp_gl.xml
@iboutle

iboutle commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Hi thanks for this;

I'm slightly nervous that this change has been written by AI and sci/tech reviewed by someone (me) who doesn't really understand the LFRic diagnostic system in-which the changes are made? But I've had a go, hopefully code-owners who are more in-the-know can check this over too?

Cheers! Mike

Don't worry, the AI has very much been supervised to produce exactly what I would have done if hand-coding it!

@MichaelWhitall MichaelWhitall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this all looks OK to me, given the extra info provided in replies to my comments :) Still pending an explanation on the optimisation change in "script_options.py" (just done as the same was done for other diagnostics in the BL explicit kernel), but Chris can cover that in his code-owner review for optimisation...

sci/tech review approved

@iboutle

iboutle commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Just grabbing this one back, as another request has come out of the woodwork that it seems sensible to add to this rather than a separate PR...

Adds a 3-D diagnostic, convection__ccw_unadjusted, giving the in-cloud
convective cloud condensed water in kg/kg before the scheme dependent
scaling that is applied to the value passed on to the radiation and
cloud schemes.

The Gregory-Rowntree scheme returns two convective cloud water
profiles: the raw in-cloud value, and a copy scaled by the deep,
mid-level and shallow tuning knobs.  Only the scaled profile is
currently available in LFRic, as convection__ccw, so the raw value is
plumbed out of conv_gr_kernel following the pattern of the existing
convection__cca_unadjusted diagnostic, and named to match it: a new
field is added to the kernel metadata, is accumulated over the
convection sub-steps only when it has been requested, and is only
accumulated on the last outer iteration.

The diagnostic is not added to the CoMorph path because CoMorph copies
its convective cloud water through unadjusted, so there the new
diagnostic would be identical to convection__ccw.

The field is added to the lfric_atm field definitions, and to the
convection kernel PSyclone transmute scripts alongside the other
optional convection diagnostics.

Some of the content of this change has been produced with the
assistance of Anthropic Claude Opus 5 (Claude Code).
@iboutle

iboutle commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

MichaelWhitall c658fef adds the "section 5" CCW diagnostic (i.e. unadjusted for radiation), which is required for some OS use (but not one currently tested in rose-stem, hence not added to the file there) - can you give it a quick check and pass on to Ed if you're happy. I've added a plot of it working from the SCM.

@MichaelWhitall MichaelWhitall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks; this looks fine to me :)

Re the decision to leave the new unadjusted CCW diagnostic out of comorph (since comorph doesn't have separate adjusted / unadjusted fields anyway)... I guess my main concern is what happens if we switch to running comorph and some downstream thing is expecting ccw_unadjusted to be set?

A crucial question is, does the lfric_apps diagnostics system have anything equivalent to STASH option codes for diagnostics in the UM? i.e. if I switch to a model configuration in-which an existing diagnostic I've requested is no-longer available, what happens? In the UM I'd get a warning telling me the STASH request has been rejected, and the field wouldn't be there in the output file, so I'd soon find out if my downstream thing tried to use the unavailable field. In LFRic, is the field similarly non-existent in the output file if I run a configuration that doesn't calculate it? Or is the field there but contains zero or junk? (that would be worrying!)

"detrain_up", "detrain_down", "dd_dt", "dd_dq", "deep_massflux",
"deep_dt", "deep_dq", "shallow_massflux", "shallow_dt", "shallow_dq",
"mid_massflux", "mid_dt", "mid_dq", "cca_unadjusted",
"mid_massflux", "mid_dt", "mid_dq", "cca_unadjusted", "ccw_unadjusted",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, it'd be nice to know / understand what the changes in these optimisation files actually do, given I'm meant to be reviewing this! (stick it to "separation of concerns"!) Are we really meant to just let AI copy/paste what's done for existing variables without understanding it?

end do
end do
end if
if (.not. associated(ccw_unadjusted, empty_real_data) ) then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I've confirmed that "it_ccw0" (which is copied into the array passed to radiation) is scaled by the ccw tuning knobs in glue_conv_6a, whereas "it_ccw" (which is copied into the new diagnostic) does not. All looks good :)

@iboutle

iboutle commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks; this looks fine to me :)

Re the decision to leave the new unadjusted CCW diagnostic out of comorph (since comorph doesn't have separate adjusted / unadjusted fields anyway)... I guess my main concern is what happens if we switch to running comorph and some downstream thing is expecting ccw_unadjusted to be set?

A crucial question is, does the lfric_apps diagnostics system have anything equivalent to STASH option codes for diagnostics in the UM? i.e. if I switch to a model configuration in-which an existing diagnostic I've requested is no-longer available, what happens? In the UM I'd get a warning telling me the STASH request has been rejected, and the field wouldn't be there in the output file, so I'd soon find out if my downstream thing tried to use the unavailable field. In LFRic, is the field similarly non-existent in the output file if I run a configuration that doesn't calculate it? Or is the field there but contains zero or junk? (that would be worrying!)

Thanks Mike - I've opened #718 to answer this question - I believe it would be better to implement a proper solution here rather than force ccw_unadjusted to be unnecessarily written when Comorph is active.

Ed Hone (@EdHone) over to you for code review (you might also have some useful thoughts on #718)

@github-actions

Copy link
Copy Markdown

⚠️ Hello iboutle!

Your CLA signature was found on the base branch, but you appear to have modified the CONTRIBUTORS.md file in this PR.

Please do not edit the CONTRIBUTORS.md file. If you have already signed the CLA, revert changes to the file and your signature will be picked up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-modified The CLA has been modified as part of this PR - added by GA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port ICAO convective cloud height and undilute CAPE diagnostics from UM to LFRic

3 participants