Skip to content

CoMorph convection scheme refactoring#292

Open
MichaelWhitall wants to merge 41 commits intoMetOffice:mainfrom
MichaelWhitall:comorph_refact
Open

CoMorph convection scheme refactoring#292
MichaelWhitall wants to merge 41 commits intoMetOffice:mainfrom
MichaelWhitall:comorph_refact

Conversation

@MichaelWhitall
Copy link

@MichaelWhitall MichaelWhitall commented Feb 25, 2026

PR Summary

Sci/Tech Reviewer: @AlisonStirling
Code Reviewer: @t00sa

closes #178

This aims to lodge the first tranch of changes to the CoMorph convection scheme code-base for CoMorph B. In preparation for numerous other tickets yet to come to lodge new science, this one just restructures various things to lay the ground work.

The key aim here is to lodge the numerous changes in code-structure from the comorph_dev branch, independent from the new science and the various changes / fixes which affect KGO. This will allow those subsequent things to be viewed as clean diffs against the trunk, making them easier to review and lodge in subsequent pull requests. While the changes to CoMorph for this pull request may look hideously large and widespread, the key thing is that they don't change answers, so they have identically preserved the existing science and technical implementation of CoMorph.

Please see linked issue #178 for full details. In particular, see these pages linked from there:

Main design changes: Issue-178-Main-Design-Changes

List of minor changes: Issue-178-Minor-Changes

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 ****

* This has been done in the process of rigorously documenting all the changes (see Issue-178-Main-Design-Changes and Issue-178-Minor-Changes). I made several changes to the code after spotting minor issues during this self-review; see recent commits.

** The branch was initially created by importing the CoMorph code-base from the equivalent UM branch (where it met the UM coding standard), and then running the LFRic code-styling script to lower-case all the fortran keywords.

*** I have checked this by viewing the compiler output from the newly-imported CoMorph standalone test, which uses gfortran with the Wall and Wextra flags turned on to catch as many issues as possible. This led to me fixing various compiler warnings, including a number that were pre-existing before this change.

**** Still awaiting the check_cr_approved test in CI, but presumably this won't complete until the code/system reviewer as approved this PR!

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)

* All rose stem tests pass.

** This pull request imports the existing set of unit tests for CoMorph and some of its components, that were already present in the UM copy of CoMorph but hadn't been lodged in LFRic yet. These tests are best run simply at the command-line rather than integrated into rose-stem or CI.

trac.log

Test Suite Results - lfric_apps - comorph_refact/run3

Suite Information

Item Value
Suite Name comorph_refact/run3
Suite User michael.whitall
Workflow Start 2026-03-04T15:57:53
Groups Run developer', 'lfric_atm_nwp_ex1a_extra', 'lfric_atm_nwp_azspice_extra
Dependency Reference Main Like
casim MetOffice/[email protected] True
jules MetOffice/[email protected] True
lfric_apps MichaelWhitall/[email protected]_comorph_refact False
lfric_core MetOffice/[email protected] True
moci MetOffice/[email protected] True
SimSys_Scripts MetOffice/[email protected] True
socrates MetOffice/[email protected] True
socrates-spectral MetOffice/[email protected] True
ukca MetOffice/[email protected] True

Task Information

✅ succeeded tasks - 1249

Security Considerations

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

Performance Impact

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

Not expecting a significant impact on performance. In some cases, simplification of the code has allowed the number of calculations to be reduced, but the impact is likely very small.

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)

Not used.

Documentation

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

Documentation for CoMorph has not yet been migrated to lfric_apps. This will be addressed in another issue / pull-request; see issue #359.

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

* The CoMorph documentation will be included in LFRic Apps as a separate issue.

(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

@MichaelWhitall MichaelWhitall self-assigned this Feb 25, 2026
@github-actions github-actions bot added the cla-required The CLA has not yet been signed by the author of this PR - added by GA label Feb 25, 2026
@MichaelWhitall MichaelWhitall added this to the Summer 2026 milestone Feb 25, 2026
@MichaelWhitall
Copy link
Author

I have also now upgraded the branch to the newly-released lfric_apps vn3.1 :) I'll rerun the rose-stem tests shortly...

…eeded to set l_positive flag passed into bad value checking in init_mass_moist_frac.
…eeded to set l_positive flag passed into bad value checking in init_mass_moist_frac.
…eeded to set l_positive flag passed into bad value checking in init_mass_moist_frac.
…eeded to set l_positive flag passed into bad value checking in init_mass_moist_frac.
Copy link

@AlisonStirling AlisonStirling left a comment

Choose a reason for hiding this comment

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

This is a long set of changes that will eventually enable new functionality (two-plumes) into CoMorph, and in many instances serve to simplify the previous code.
Clear explanations for the changes accompany this issue, and crucially, and by design, they don't change answers across the range relevant tests.
A significant addition to the new code lines is the inclusion of unit tests on CoMorph's standalone model, which is a very useful, rapid tool for testing the scheme.
We note that in the past day or so there has been an announcement on documentation pages for LFRic APPs. The porting of documentation will be done as a separate issue.
@t00sa I am happy to approve this, and pass to code review.

@github-actions github-actions bot requested a review from t00sa March 11, 2026 14:35
@james-bruten-mo james-bruten-mo removed their request for review March 12, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CoMorph convection scheme refactoring

4 participants