Conversation
paulfield2024
left a comment
There was a problem hiding this comment.
fixes a bug that allowed Nans to propagate
There was a problem hiding this comment.
Line 153 is: dmass_d=0.0 - so I would dispute that it was previously uninitialised. I'm happy to relocate it to L265 though - since it's closer to where it is actually used, and the use of a parameter rather than 0.0 is preferred. Can we remove L153 though please?
L276-277 else statement is redundant.
I think L153 is needed, because L265 is within if-statements that don't always get executed. Presumably that means L265 isn't actually needed (I presume I've done this and removed the |
Erica Neininger (ericaneininger)
left a comment
There was a problem hiding this comment.
Thanks for these changes.
PR Summary
Sci/Tech Reviewer: paulfield2024
Code Reviewer: Erica Neininger (@ericaneininger)
Fix for uninitialised values of
dactiveanddmass_din activation.Linked to MetOffice/lfric_apps#709 and MetOffice/um#117
N.B. submitted as a triple-linked change to make the dependencies clear. The Casim change will have no effect on the lfric_apps trunk as-is, but is a requirement for the lfric_apps ticket to function. So the Casim ticket could be committed and UM KGO updated, followed by the apps ticket being committed later.
Code Quality Checklist
(Some checks are automatically carried out via the CI pipeline)
Testing
stem suites
acceptable (eg. kgo changes)
tests, unit tests, etc.)
trac.log
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
Sci/Tech Review
Please alert the code reviewer via a tag when you have approved the SR
Code Review