-
Notifications
You must be signed in to change notification settings - Fork 161
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
Refactor surface restart logic in FV3GFS_io.F90 to not use hard-coded indices. #605
Refactor surface restart logic in FV3GFS_io.F90 to not use hard-coded indices. #605
Conversation
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'm not too familiar with this code, but the PR is well documented and the code looks clean.
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.
@SamuelTrahanNOAA Very useful modifications to FV3GFS_io.F90 to remove hard-wired numbers! This subroutine used to be a culprit when some variables have to be added to the input or restart file. Thank you very much, Sam, for your tremendous work to make this subroutine more flexible.
@SamuelTrahanNOAA Please update 'atmos_cubed_sphere' submodule to 16e659e5 hash from my branch. |
d0dbb21
to
94c955e
Compare
All tests are done on ufs-community/ufs-weather-model#1497. We can start merging in this pr. |
Description
The surface restart uses hard-coded indices, which makes it difficult to add new restart variables, especially if different land surface schemes need different numbers of variables. This is problematic for developer @tanyasmirnova who has that exact problem.
The code now uses a variable
nt
(number of things) it increments before copying to or from a slice of a surface field array. Most of the copies happen in copy_to_GFS_Data or copy_from_GFS_Data, which wraps this into a tight, vectorizable, loop, while also making the syntax concise.That means you'll see two constructs a lot.
Here, you see two columns: one increments nt, and one does the assignment. By having the two statements on one line, it is easy to spot if there's a missing
nt=nt+1
:Most of the slices are copied like so:
where the
nt=nt+1
and data copying logic is hidden behind the subroutine.There were a few odd copies, with bizarre indexing (-2:4) or ones with calculations. For those, the logic is made explicitly clear:
The code is also refactored to have the loops in an order that should be faster and easier to vectorize: nb, variable, k, ix.
When the code calls copy_to_GFS_Data or copy_from_GFS_Data, the k and ix loops are hidden behind subroutines:
Issue(s) addressed
fixes #604
Testing
hera.intel and hera.gnu ufs weather model regression tests