-
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
Add parallel netcdf read/write from EnKF for sfc files (paranc option) #707 #709
Conversation
@tsga , two questions
|
@RussTreadon-NOAA, I think @ClaraDraper-NOAA and @jswhit would be the right people to do the reviews. I have WCOSS account, but am having hard time logging into it. I will post the tests here as soon as I manage to run them. |
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.
@tsga A few small changes. Also, if you have not already, can you please run the soil analysis, using the parallel and non-parallel read/write versions, and check that the results are the same? Should we expect bit-compatability?
This reverts commit fca6bea.
This reverts commit fca6bea.
This reverts commit 379d7e4.
* origin: Revert "Revert "CrIS viirs bug fix (NOAA-EMC#708)"" Revert "CrIS viirs bug fix (NOAA-EMC#708)" CrIS viirs bug fix (NOAA-EMC#708)
… feature/paranc_sfc * 'feature/paranc_sfc' of https://github.com/tsga/GSI: Revert "CrIS viirs bug fix (NOAA-EMC#708)"
@ClaraDraper-NOAA thank you. I have now made the changes you recommended. |
@RussTreadon-NOAA the "global_4denvar" test keeps failing due to "permission denied on file access". The full log file is at: /lfs/h2/emc/da/noscrub/tseganeh.gichamo/GSI/logerr_global_4denvar |
@tsga , file
To learn more about restricted data, including how to request access, go to https://www.nco.ncep.noaa.gov/pmb/docs/restricted_data/ |
@RussTreadon-NOAA, thank you! I got added to rstprod, and the test passes now. |
Great, thanks. |
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 only looked at syntax, not science. Changes look OK.
The existing global_enkf ctest does not exercise the new paranc option, right? Should consider updating global_enkf at some point to ensure the new functionality added by the PR is not broken by subsequent PR.
@RussTreadon-NOAA paranc is used in operations, so I'd expect it to be used in the existing ctests. what this PR does is allow us to use paranc with the soil analysis (up to now, I've just been turning it off). I haven't yet added a ctest for the soil analysis, but that's where we'd want to test this code ( I think?). |
Thank you @ClaraDraper-NOAA, could we turn on soil analysis for this case by changing namelist options and updating input files? global_enkf runs a 10 member ensemble at C48L127. If setting up a soil analysis test isn't trivial, it be captured in a new issue and subsequent PR. |
Adding a c-test is probably a good idea. I've not added a c-test to GSI before, but I don't think it'd be too hard. The output of turning on the soil analysis is a sfc increment file, with soil moisture and soil temperature increments. The input is changed convinfo and anavinfo files, and some namelist variables. @tsga - my global_workflow PR has been merged, you can pull the necessary changes from that. |
@ClaraDraper-NOAA and @tsga , we do not want to add another ctest. GSI ctests are actually regression tests. They are not quick and fast. If we can turn on the soil analysis in the current C48L127 global_enkf case, great. If not, we can open a new issue to update the global_4denvar and global_enkf ctests to mimic GFS v17. The GSI needs modifications (issue #719) to run with Thompson microphysics. A new C96/C48L127 GFS v17 case could be created to cover both soil analysis (global_enkf) and atmospheric DA (global_4denvar). |
@tsga , are you done working on |
@RussTreadon-NOAA, yes I am done. Thank you. |
One final run of ctests on WCOSS2 (Cactus) with the following results
|
Orion ctests
While the updat times are considerably higher than contrl, the The
A check of
The hiproc_updat ran faster than the contrl. The opposite is true for the loproc jobs. The
The
The hiproc wall times are comparable. The updat wall time is considerably higher than contrl for the loproc configuration. While not desirable, none of these failures are regarded as fatal given the impact the Orion |
In that case, I suggest we open a new issue to update the two tests that you identified. Together with the new microphysics works for me. |
@ShunLiu-NOAA , @hu5970 , and @CoryMartin-NOAA : This PR is ready for merger into Clara & Jeff reviewed and approved the chagnes. Ctests run and yield acceptable results on WCOSS2 (Cactus), Hera, and Orion. |
DUE DATE for merger of this PR into develop is 4/8/2024 (six weeks after PR creation).
Description
Currently, when the soil analysis is used we turn off parallel read/write, since the parallel routines to read/write the soil states and increments have not been coded. However, the default in operations is to do parallel read/write (paranc=.true.).
This "feature add" enables writing land states read and write in parallel (or in serial as in the past) based on user config.
Fixes #707
Type of change
How Has This Been Tested?
The 7 regression tests have passed on hera.
Start testing: Feb 26 16:52 UTC
1/7 Testing: global_4denvar
Test time = 2711.52 sec
Test Passed.
2/7 Testing: rtma
Test time = 1999.66 sec
Test Passed.
3/7 Testing: rrfs_3denvar_glbens
Test time = 3678.76 sec
Test Passed.
4/7 Testing: netcdf_fv3_regional
Test time = 672.00 sec
Test Passed.
5/7 Testing: hafs_4denvar_glbens
Test time = 1825.27 sec
Test Passed.
6/7 Testing: hafs_3denvar_hybens
Test time = 2484.58 sec
Test Passed.
7/7 Testing: global_enkf
Test time = 2749.58 sec
Test Passed.
End testing: Feb 26 21:21 UTC
Checklist