Allow different meshes to have different maximum halo depths#174
Allow different meshes to have different maximum halo depths#174tommbendall wants to merge 5 commits intoMetOffice:mainfrom
Conversation
jameskent-metoffice
left a comment
There was a problem hiding this comment.
This work allows the different meshes to have different maximum halo depths. This means that the multigrid meshes do not have to have the same size halo depths as meshes used for transport.
There is a check whether the coarser meshes will be used for aerosol transport, which is good. The rest of the code is good. There is one small suggested alignment which I've put in the code.
The plot for the kgo changing test looks good, as does the explanation about the halo size in the 2D X-Z tests. I am happy this passes science review.
applications/lfric2lfric/source/initialisation/lfric2lfric_init_mesh.f90
Show resolved
Hide resolved
|
Thanks for the science review. @allynt this is now ready for code review |
james-bruten-mo
left a comment
There was a problem hiding this comment.
Allyns on leave this week, so I've had a look at these PRs
No issues with the changes, but there look to be a few merge conflicts if you could take a look at sorting those
Cheers
|
Your CLA signature was found on the base branch, but you appear to have modified the CONTRIBUTORS.md file in this PR. Please do not edit the CONTRIBUTORS.md file. If you have already signed the CLA, revert changes to the file and your signature will be picked up. |
|
I've just merged in main and resolved conflicts. Should I also regenerate the KGOs? |
I'm not too worried - I'll be regenerating them regardless |
|
I don't think this change requires an update on the @DanStoneMO - would you mind doing that? Thankyou :-) |
PR Summary
Sci/Tech Reviewer: @jameskent-metoffice
Code Reviewer: @james-bruten-mo
Our current infrastructure sets a single maximum halo depth for all meshes, irrespective of their relative resolution. This isn't is necessary and doesn't match the use of the meshes. In typical atmospheric configurations, the finest mesh needs a depth appropriate for the transport scheme, which is set by the expected maximum Courant number (e.g. 10). In contrast, the coarser meshes used for multigrid generally only need a halo depth of 2, and yet we still set their depth to 10. For these multigrid meshes, this means the halos can end up extending beyond the adjacent partitions (unnecessarily!)
This is a linked PR, which addresses this by allowing different halo depths to be set for different meshes. The change in this PR is:
get_required_stencil_deptha subroutine, returning the value for a particular meshget_required_stencil_depthand replacing it withmesh%get_halo_depth()in appropriate science codeinit_meshroutine, so thatstencil_depthis a rank-1 array (with different values passed for different meshes)This changes the KGO of the Schar mountain test. This is because this test has a width in the y-direction of 8 cells, but uses a maximum halo depth of 4 -- meaning that the furthest cell in both positive and negative directions is the same. This triggers issue #175. A separate PR should change those vertical slice tests to avoid this issue.
Linked to: MetOffice/lfric_core#237
Plot of test with changed KGO:

Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - stencil_depths/run8
Suite Information
Task Information
✅ succeeded tasks - 1456
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