PC2 response to horizontal mixing#464
Conversation
James Bruten (james-bruten-mo)
left a comment
There was a problem hiding this comment.
Macro all fine
DanStoneMO
left a comment
There was a problem hiding this comment.
Can confirm JEDI works fine with this, no linked PR is needed for lfric-jedi
MichaelWhitall
left a comment
There was a problem hiding this comment.
Hi thanks lots for implementing this change! I think I'm broadly happy with this, just a few questions below...
Another more general thing is how much work would it be to add diagnostics for the diffusion increments (excluding PC2) and the PC2 homog increments added by this PR? Given this is not something I can just eyeball in the SCM, it would be nice to be able to look at example maps of the increments at some height where there's cloud and check that they look as expected?
I've added the diagnostics as requested, and changed the test job. Back to MichaelWhitall for science review |
|
The added stuff to output the increment diagnostics look good to me; wow it really is a lot less work to add these in LFRic compared to the UM! I'm happy with this now, approving the sci/tech review... |
|
Oakley Brunt (@oakleybrunt) ready for code review |
Oakley Brunt (oakleybrunt)
left a comment
There was a problem hiding this comment.
Just one comment about adding timing callipers. Otherwise, happy with this change.
Co-authored-by: Oakley Brunt <130391369+oakleybrunt@users.noreply.github.com>
Co-authored-by: Oakley Brunt <130391369+oakleybrunt@users.noreply.github.com>
Co-authored-by: Oakley Brunt <130391369+oakleybrunt@users.noreply.github.com>
Co-authored-by: Oakley Brunt <130391369+oakleybrunt@users.noreply.github.com>
Co-authored-by: Oakley Brunt <130391369+oakleybrunt@users.noreply.github.com>
Oakley Brunt (oakleybrunt)
left a comment
There was a problem hiding this comment.
Thanks!
PR Summary
Sci/Tech Reviewer: MichaelWhitall
Code Reviewer: Oakley Brunt (@oakleybrunt)
Closes #242
When using the PC2 cloud-scheme in model-configurations which include horizontal turbulent mixing (aka 3D Smagorinsky or blended boundary-layer), currently the horizontal mixing calculation does not update the PC2 cloud-fraction prognostic consistent with cloud water content. This will be allowing the cloud water and fraction prognostics to become inconsistent with eachother, which can cause numerical problems elsewhere (e.g. increasing cloud water without increasing fraction can lead to silly large in-cloud water contents, which rain-out spuriously).
For vertical turbulent mixing (aka the boundary-layer scheme), PC2 already updates the cloud water content and fraction consistently by assuming the mixing increment acts as a homogeneous forcing. For now we will just implement the same approach for the horizontal mixing, for consistency.
I have switched on Smagorinsky + the new option in the comorph_dev test to protect the new code, hence this has changed KGOs. The KGO change to other jobs on the ex1a arises because we have split up a single line of code that multiplied the Smagorinsky increment by dt and added it back to the main increment into 2 lines - the multiplication by dt needs to happen before the PC2 code is called, and adding the increment back needs to happen after this.
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - pc2_smag/run1
Suite Information
Task Information
✅ succeeded tasks - 1310
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review