-
Notifications
You must be signed in to change notification settings - Fork 28
Small fixes to better enable running with XIOS 3 #218
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
base: main
Are you sure you want to change the base?
Conversation
mo-marqh
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.
i have sci-tech reviewed these changes.
the buffer_size¬factor is a trivial mistake fix that was ignored in XIOS2 but fails in XIOS3. So, this was always wrong. Now updated to correct for both versions.
The setting of read_access to true is essential for XIOS3 and neutral for XIOS2, the implementation is clean, with the extension of a simple xios_set_attr call to include the logical from the lfric-xios object.
|
@mo-lottieturner this change passes sci/tech and is now ready for code review. (it's a stand-alone lfric_core change) |
mo-marqh
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.
hi @EdHone
it would be helpful if the setting of read_access was also enabled for lfric_xios created checkpoint definitions when actually restarting
e.g.
--- a/components/lfric-xios/source/lfric_xios_metafile_mod.F90
+++ b/components/lfric-xios/source/lfric_xios_metafile_mod.F90
@@ -269,6 +269,11 @@ subroutine add_field(metafile, dict_field_id, mode, operation, id_as_name, legac
end if
end if
end if
+ if (mode == RESTARTING) then
+ ! Is restarting, then set read_access to True.
+ call xios_set_field_attr(field_id, read_access=.true.)
+ end if
+
|
reverting back to sci/tech review based on recent reanalysis |
|
Branch update with the change to enable Passing back to you @mo-marqh and then on to @mo-lottieturner |
PR Summary
Sci/Tech Reviewer: @mo-marqh
Code Reviewer: @mo-lottieturner
This PR fixes a few issues in our code that are fine with XIOS2 but fail with XIOS3, namely:
buffer_size_factorconfig parameterreadmode files which do not haveread_access=.true.This PR fixes them and the fixes are backwards compatible with XIOS 2.
With the additional changes from #204, the lfric-xios technical tests are able to be run with both XIOS2 and XIOS3.
Code Quality Checklist
(Some checks are automatically carried out via the CI pipeline)
style guidelines
readability of the code
Testing
using this branch
acceptable (e.g. kgo changes)
tests, unit tests, etc.)
and have 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_core - lfric_core-215-xios3-fixes/run2
Suite Information
Task Information
✅ succeeded tasks - 372
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
interface, optimisation scripts, LFRic data structure code) then please
contact the
tooscollabdevteam@metoffice.gov.uk
Sci/Tech Review
Please alert the code reviewer via a tag when you have approved the SR
Code Review