CoMorph further KGO-preserving refactoring - #717
MichaelWhitall wants to merge 38 commits into
Conversation
…ph_refact2 and ran the lfric_styling.py script.
… the fix which stops the check-cla test from repeatedly spuriously failing, as it annoyingly spams my email every time this happens).
|
Sci tech review approved |
…d parcel radius option, correct sub-grid fraction treatment for graupel in the stand-alone comoprh test.
…amelist block in the rose metadata.
…ck. Try use-ing them from comorph_config_mod?
… case where namelist:convection (and hence namelist:comorph too) are not in brackets as well as the existing brackets case (the ngarch rose-stem job doesn't have the brackets so the existing macro breaks it).
… the match as that's not there in source after parsing by self.get_setting_value).
…the various example configuration .nml files (none of these use comorph so the new namelist doesn't need to be added to any of them.
…ackets even if namelist:convection isn't, to indicate that its optional.
… and just specify the whole string to be added (1 preceding space at start, newline at end) instead of matching the previous line.
…or code-style / comment changes, (b) moved autoconversion code from microphysics_2 inside its own subroutine, (c) moved setting of core_ent diagnostic from conv_level_step inside set_ent, (d) added / corrected chk_var calls in comorph_um_namelist_mod...
DrTVockerodtMO
left a comment
There was a problem hiding this comment.
Adjoint config changes are fine, thank you!
…ctually are (sci/tech review request).
…ctually are (sci/tech review request).
…ibility of multiple condensate species contributing to the cloud-fractions (needed when switching on the 2nd ice category for use with CASIM). Also deleted stray module use of set_par_cloudfrac from conv_level_step (it isn't used in that routine yet).
|
Rose-stem test results / trac.log output on the PR summary now updated with a run from the latest branch revision; all tests still pass :) |
AlisonStirling
left a comment
There was a problem hiding this comment.
Sci/Tech reviewed after additional code added since 21st Aug 2026
…t test for phase_change_solve, and a few more code-style changes to harmonize with the comorph_dev branch.
|
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. |
|
Sorry just added a last-minute extra commit:
|
… double to reproduce KGO in MetOffice#717; now we're breaking the KGOs anyway we may-as-well set this the same as the rest of the comorph namelist for consistency.
…an the Issue number, for consistency with MetOffice#717 (now that we have the PR number for this one).
AlisonStirling
left a comment
There was a problem hiding this comment.
Sci /tech approved extra submission from Friday 11th September.
|
James Bruten (@james-bruten-mo), cjohnson-pi, Hacka Fett (@christophermaynard), Steven Sandbach (@ss421), Matt Shin (@matthewrmshin), any chance you can look at this PR Re code-owner approval? For most of you, its just that it modifies an example configuration file you own to keep it consistent with the namelists / meta-data. Among other things, this PR moves some comorph convection scheme settings from the "convection" namelist to a new "comorph" namelist. It turns out none of the example configurations use comorph, so rather than adding the comorph namelist to the Note I'd like to get this PR lodged soon if possible as it blocks another PR... Cheers! |
|
Ed Hone (@EdHone), this PR is ready for code-review to begin :) Note this one blocks PR #776 which is also aimed at the next release so it'd be nice to get it lodged relatively soon if that's possible? (still pending some code-owner approvals but I've sent a reminder to the affected code-owners just now...) Cheers! |
James Bruten (james-bruten-mo)
left a comment
There was a problem hiding this comment.
Macro approved
We store namelist configurations in the JEDI testing and these are based on the configurations in LFRic which is ultimately stored in the rose configuration files. Its easier to see what has changed in the namelist files but these should mirror the ones in the rose configuration. I agree that having a mechanism to automatically do that via the rose configs but we are not able to do that just yet so we have this pain for now. From what I understand in the PR, it seems that while the namelist files have been updated that is related to changes that had not been applied previously and not this PR? |
Thanks; the changes to the example configurations in this PR actually just undo the changes from a previous PR which unnecessarily (technically wrongly) added the convection namelist items I had to remove Also, I now realise most of the example config files (and those in JEDI) don't use most of the physics. As long as namelist / meta-data changes only affect things that would be trigger-ignored anyway when not using the relevant physics, then no update to the example configs in JEDI will be needed. Cheers! |
Steven Sandbach (ss421)
left a comment
There was a problem hiding this comment.
Thanks. Dan has checked and everything works for us :-)
PR Summary
Sci/Tech Reviewer: AlisonStirling
Code Reviewer: Ed Hone (@EdHone)
The bulk of the refactoring of CoMorph in the comorph_dev branch was lodged in the lfric_apps trunk in Issue #178 / PR #292. But some more KGO-preserving refactoring has been done in the comorph_dev branch after the branch lodged for #292 was split-off from it. We now need to lodge these extra refactoring changes. We also need to add more of CoMorph's tune-able constants to the LFRic namelist.
For a detailed description of the changes, see the Issue page: #713.
Since we've altered the namelists, this PR requires an upgrade macro to apply the namelist changes to existing apps, and a test branch in-which the upgrade macro has been applied to rose-stem: comorph_refact2 -> comorph_refact2_test
closes #713
blocks #776
Code Quality Checklist
**Thecheck-clatest is failing, even though I have not modified thecontributors.mdfile in this PR (and my name is already added to it on the trunk). This test just seems to be broken.Testing
Note: while all the comorph apps in the lfric_apps rose-stem tests preserved KGO exactly (as did all the UM rose-stem tests with the same changes to comorph in that model), when I tested the same changes in UM N96 AMIP runs (outside of rose-stem) there was in-fact a change in answers for the CoMA9_tb configuration. The difference appears to be insignificant / science-neutral, and is presumably due to a change in compiler optimisation behaviour (the AMIP run uses different compiler optimisation settings to the rose-stem tests). More detail of the tests performed and the results can be seen here:
Testing CoMorph refactoring and bug-fixes
trac.log
Test Suite Results - lfric_apps - comorph_refact2_test/run3
Suite Information
Task Information
✅ succeeded tasks - 1309
Security Considerations
Performance Impact
No change in model performance expected.
AI Assistance and Attribution
A Met Office Github Copilot Enterprise AI agent was used in visual studios code to do the following:
Documentation
Porting comorph documentation to lfric_apps is still tbd, see #359.
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review