Skip to content

Fix hm snow positivity - #24

Open
Paul J. Connolly (maul1609) wants to merge 3 commits into
MetOffice:mainfrom
UoM-maul1609:fix-hm-snow-positivity
Open

Paul J. Connolly (maul1609) wants to merge 3 commits into
MetOffice:mainfrom
UoM-maul1609:fix-hm-snow-positivity

Conversation

@maul1609

Copy link
Copy Markdown

PR Summary

Fix snow positivity limiting for Hallett–Mossop

Sci/Tech Reviewer: paulfield2024
Code Reviewer:

Hallett–Mossop removes snow mass but was in the non-scalable list in the snow ensure_positive call, so it could make snow negative. This moves i_ihal to the scalable list. Also adds me to CONTRIBUTORS.md.

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

Testing

  • If shared files have been modified, I have run the UM and LFRic Apps rose stem suites
  • If any tests fail (rose-stem or CI) the reason is understood and acceptable (eg. kgo changes)
  • I have added tests to cover new functionality as appropriate (eg. system tests, unit tests, etc.)

Compiles with gfortran. Rose stem not run; KGO changes expected where the snow limiter is active.

Security Considerations

  • I have reviewed my changes for potential security issues

Performance Impact

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

AI Assistance and Attribution

  • Not produced with AI

Documentation

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

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

hallet_mossop removes rimed mass from snow (procs(snow%i_1m, i_ihal) =
-dmass_s), but i_ihal was passed to ensure_positive for snow in the
non-scalable list.  Non-scalable processes are never limited, so when
the other snow sinks are small, rime splintering could take snow mass
below zero.

Move i_ihal to the scalable list for snow.  ensure_positive rescales all
species of a process together, so the ice source from splintering is
reduced consistently with the snow sink.  Results change only in grid
boxes where the snow positivity limiter is active.
@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 Oct 9, 2026
@github-actions
github-actions Bot requested a review from paulfield2024 October 9, 2026 14:14
@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 Oct 9, 2026
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.

2 participants