-
Notifications
You must be signed in to change notification settings - Fork 150
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
Optimize the reading of ensembles and setup for global multiscale runs #594
Conversation
…FV3 DA (FV3LAMDA)
Updated to head of trunk and remove commented out line from build.sh. |
Regression tests were rerun with this updated version of the code. All regression tests passed except 4denvar. The last update to the trunk appears to have introduced a very small change into the 4denvar. This difference is certainly at the scale of round-off. No difference in the initial penalty. First 3 iterations of the control and update are given below. < cost,grad,step,b,step? = 1 0 6.592319865747931181E+05 1.700824233239468640E+03 1.057506532852620307E+00 0.000000000000000000E+00 good
Trying to find reason for small difference. |
WCOSS2 ctests
The
A check of the task 0 maximum resident set sizes for the updat (
The The
A check of the wall times shows that the
This is consistent with the optimization purpose of this PR. This is not a fatal fail. The non-reproducible results between the
John found similar behavior in his tests. |
orking on the nonreproducible issue. I have not found anything that I changed that should change the round-off or anything. Hope to find something soon. Surprised at the large increase in memory. Will look at it after finding the nonreproducible issue.
|
@jderber-NOAA , when you have time would you please update |
Reason for reproducibility issue found. It was documented earlier in this development. With the change of 3 lines in get_gefs_ensperts_dualres.f90 (around line 190) all regression tests passed. While looking for the reproducibility problem a few changes were made.
Regression tests were rerun. All passed except rrfs_3denvar_glbens. Not sure why this did not pass. Results were the same. Run times were faster. Slightly more memory. This must have been the reason for the failure. But results are reasonable. Updating to head of trunk. |
WCOSS2 ctests
The
Examination of the updat and contrl wall times does not show any anomalous behavior
This is not a fatal fail. |
After updating to the head of the trunk (both the control and updat), all regression tests passed on Hera. Test project /scratch1/NCEPDEV/da/John.Derber/converge4/GSI/build 100% tests passed, 0 tests failed out of 9 Total Test time (real) = 1813.71 sec I will put back in the code the 3 lines that do give a minor difference. These lines result in fewer conversions between variables and should produce slightly more consistent results. |
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.
Approve pending peer reviews
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.
Finished another a test using 3km conus domain RRFS case. This PR gives the identical results (final cost and gradients) compared with the EMC GSI trunk .
Thanks for this continual improvement over GSI
A note: in my RRFS run, it is found there are little differences between the GSI of this PR built with or not with debug mode.
It is , for GSI built with "realease" mode:
EMC GSI trunk shows the same behavior. Namely, in debug mode, this PR and EMC GSI trunk show the identical results for build type : Release or debug and the above tiny differences exist between Release mode and debug mode for both branches. |
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.
Looks great. Thanks @jderber-NOAA!
@TingLei-NOAA Thanks for looking into that reproducibility issue. If this is also an issue in the develop branch, I don't see a need to hold up this PR.
@CatherineThomas-NOAA Agree! |
After latest update to head of the trunk, regression test run. Results as expected. The update did not impact changes. |
Given the following
proceed to merge |
This update improves the efficiency of the GSI, especially for multiscale runs. Details can be found in issue#585
The runs produce identical results except when ensembles are used. Identical results can be produced with ensembles as well with changes to 3 lines of code. These lines zero out negative moistures before creating virtual temperatures and use the original sensible temperatures rather than ones created from virtual temperatures (which were created from the original sensible temperatures).
All regression tests passed except due to the above reason. When those 3 lines were changed back, all regression tests passed. Changes due to the above 3 lines were very minor.
All testing was performed by myself on Hera.
See Issue to see examples of speed-ups of the code that resulted from this change.
Fixes #585
Checklist
DUE DATE for this PR is 9/6/2023. If this PR is not merged into develop by this date, the PR will be closed and returned to the developer.