-
Notifications
You must be signed in to change notification settings - Fork 63
Correct the sample_physics_winds_correction option #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correct the sample_physics_winds_correction option #69
Conversation
atb1995
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code fixes the current failure on main whilst using the sample_physics_winds_correction option - passing a field of the correct stencil depth and fixing the w3_to_w2_correction kernel in the linked PR by not using a vertical level index on a 2D field. Finally, it tidies/fixes the misused previous logic for application of the correction. I'm happy that the code is well written and does what was stated in the PR.
|
Thanks for the science review, @allynt this is now ready for code review |
allynt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR Summary
Sci/Tech Reviewer: @atb1995
Code Reviewer: @allynt
The
sample_physics_winds_correctionoption is broken (see Issue #66). This had not been detected as it is not currently covered by the test-suite.This PR fixes this option and improves on the current situation by making the following changes:
input fields to the
w3_to_w2_correction_kernelare declared to depth 2: this kernel reads from a field with a depth 1 halo and is an INC kernel, so fields being read using stencils must be declared to depth 2 (they are only depth 1 by default)I have unified the
sample_physics_winds_correctionoption with the other code to perform thesample_physics_winds: thesample_physics_winds_correctionoption should simply activate the correction routines, but it previously also changed the method of samplingit picks up a correction to the
w3_to_w2_correctionkernel from the Linked PR: Fix Correction to Sampling Physics Winds lfric_core#221I have turned this option on in an exoplanet rose-stem test to ensure that it is covered by the test-suite
Linked to: Fix Correction to Sampling Physics Winds lfric_core#221
To demonstrate that this is working, I have created an artificial test in which a zonal wind component in W3 was specified to vary with latitude. This was then mapped to W2, with and without the correction option. The following plots show that grid imprinting on the meridional panel edges has been improved.
See the small divots on the meridional panel edges:

And zoomed in on a panel edge:

Code Quality Checklist
(Some checks are automatically carried out via the CI pipeline)
style guidelines
readability of the code
Testing
acceptable (eg. kgo changes)
tests, unit tests, etc.)
and have tests 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)
trac.log
Test Suite Results - lfric_apps - smp_phys_wind_correct/run1
Suite Information
Task Information
✅ succeeded tasks - 1256
Security Considerations
Performance Impact
performance measurements have been conducted
AI Assistance and Attribution
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)
Documentation
confirmed that it builds correctly
PSyclone Approval
inteface, optimisation scripts, LFRic data structure code) then please
contact the
[email protected]
Sci/Tech Review
Please alert the code reviewer via a tag when you have approved the SR
Code Review