Skip to content

Only use mp_czero_rp if using the RP2B scheme#482

Open
Marc Stringer (marcstring) wants to merge 2 commits into
MetOffice:mainfrom
marcstring:mp_czero_fix
Open

Only use mp_czero_rp if using the RP2B scheme#482
Marc Stringer (marcstring) wants to merge 2 commits into
MetOffice:mainfrom
marcstring:mp_czero_fix

Conversation

@marcstring
Copy link
Copy Markdown

@marcstring Marc Stringer (marcstring) commented May 6, 2026

PR Summary

Sci/Tech Reviewer: yongmingtang
Code Reviewer: Cameron Bateman (@cameronbateman-mo)

MetOffice/um#73 describes how UM ticket #6980 (https://code.metoffice.gov.uk/trac/um/ticket/6980) broke the option l_rp2=true and i_rp_scheme=0, which is used by our prototype UKESM2 model. Our prototype UKESM2 uses the UM, but during the approval process Paul Barrett has pointed out that the fix in MetOffice/um#73 should also be applied to LFRic, hence this pull request.

MetOffice/um#73 modifies three FORTRAN files: large_scale_cloud/bm_calc_tau.F90, large_scale_precipitation/mphys_turb_gen_mixed_phase.F90 and stochastic_physics/stph_rp-stph_rp2.F90. The first two files can be found in lfric_apps, but not the third (stochastic_physics/stph_rp-stph_rp2.F90), so I've only mirrored the changes from MetOffice/um#73 for large_scale_cloud/bm_calc_tau.F90 and large_scale_precipitation/mphys_turb_gen_mixed_phase.F90 for the lfric_apps branch here.

I don't work on LFRic and I'm not aware of any LFRic jobs which can be used to test the option l_rp2=true and i_rp_scheme=0, so my testing for lfric_apps is limited to just running rose-stem, which passes fine.

  • linked MetOffice/um#73

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)
  • [ ]*1 I have added tests to cover new functionality as appropriate (e.g. system tests, unit tests, etc.)
  • [ ]*2 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)
    *1 No extra tests here. Protection of the option l_rp2=true and i_rp_scheme=0 is being added to our UKESM2 UM rose-stem jobs (UKESM-curr & UKESM-beta).
    *2 Not applicable

trac.log

Test Suite Results - lfric_apps - test_lfric_mp_czero_fix/run1

Suite Information

Item Value
Suite Name test_lfric_mp_czero_fix/run1
Suite User marc.stringer
Workflow Start 2026-05-06T08:42:38
Groups Run all
Dependency Reference Main Like
casim MetOffice/casim@2026.03.2 True
jules MetOffice/jules@2026.03.2 True
lfric_apps marcstring/lfric_apps@mp_czero_fix False
lfric_core MetOffice/lfric_core@2026.03.2 True
moci MetOffice/moci@2026.03.2 True
SimSys_Scripts MetOffice/SimSys_Scripts@2026.03.2 True
socrates MetOffice/socrates@2026.03.2 True
socrates-spectral MetOffice/socrates-spectral@2026.03.2 True
ukca MetOffice/ukca@2026.03.2 True

Task Information

✅ succeeded tasks - 1511

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

  • [x]*3 Performance of the code has been considered and, if applicable, suitable performance measurements have been conducted
    *3 This is just expanding a couple of if statements for routines not called that often. Impact will be negligible.

AI Assistance and Attribution

  • [ ]*4 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)
    *4 No AI here

Documentation

  • [ ]*5 Where appropriate I have updated documentation related to this change and confirmed that it builds correctly
    *5 No documentation update required

PSyclone Approval

  • [ ]*6 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
    *6 Not applicable

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

@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 May 6, 2026
@marcstring Marc Stringer (marcstring) added the Linked UM This PR is linked to a MetOffice/um PR label May 6, 2026
@github-actions github-actions Bot added cla-signed The CLA has been signed as part of this PR - added by GA and removed cla-required The CLA has not yet been signed by the author of this PR - added by GA labels May 6, 2026
@github-actions github-actions Bot requested a review from yongmingtang May 6, 2026 11:03
@marcstring
Copy link
Copy Markdown
Author

i_rp_scheme=0 scheme unavailable at LFRic

In https://code.metoffice.gov.uk/trac/UKESM/wiki/ticket/1101/lfric_converse I've recorded an e-mail conversation between me and Anne McCabe where she says that the i_rp_scheme=0 is unavailable at LFRic, which explains why there's no stochastic_physics/stph_rp-stph_rp2.F90 in LFRic apps.

Copy link
Copy Markdown

@paulfield2024 paulfield2024 left a comment

Choose a reason for hiding this comment

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

seems fine. Passes rose-stems

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

seems fine, as UM

Copy link
Copy Markdown

@yongmingtang yongmingtang left a comment

Choose a reason for hiding this comment

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

all is fine -- approved

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 Linked UM This PR is linked to a MetOffice/um PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants