-
Notifications
You must be signed in to change notification settings - Fork 155
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 FED EnVar DA Capability #632
Conversation
2. TL/AD related codes test and debug 3. refine codes
2. Use 3D interplotion when FED from BG is used 3. Clean code modified: src/gsi/gsimod.F90 modified: src/gsi/obsmod.F90 modified: src/gsi/read_fed.f90 modified: src/gsi/setupfed.f90
modified: ../src/gsi/setupfed.f90
modified: src/gsi/setupfed.f90
@hu5970 @TingLei-NOAA |
@TingLei-NOAA @wangym1111 Could you please wait until next Monday to start the review? I may have some update this week. Thanks for your time and effort! Hongli |
modified: src/gsi/control2state.f90 modified: src/gsi/cplr_get_fv3_regional_ensperts.f90 modified: src/gsi/ensctl2model_ad.f90 modified: src/gsi/ensctl2state.f90 modified: src/gsi/intfed.f90 modified: src/gsi/stpfed.f90
@hongli-wang Could you sync with develop branch? I will do regression tests on WCOSS2 after syncing. |
@hu5970 Hongli |
Regression tests on WCOSS2 finished:
The failure of "netcdf_fv3_regional" in the first run is because over allowable memory. So, All regression test cases passed. |
@hu5970 Would you please look at my test: |
Your "hafs_4denvar_glbens" is OK. It failed at "time-thresh": But "rrfs_3denvar_glbens" failed during start of the first run. Please rerun this one to see if you can repeat the crash. |
@hu5970 @TingLei-NOAA /scratch1/BMC/wrfruc/hwang/ctest/GSI 1/1 Test #3: [=[rrfs_3denvar_glbens]=] ........***Failed 61.64 sec 0% tests passed, 1 tests failed out of 1 Total Test time (real) = 61.66 sec The following tests FAILED: forrtl: severe (71): integer divide by zero |
@hongli-wang The crash point is related to the number of cores used for test. It does not make sense to me why it crashed. |
I run regression test on Hera also. All cases passed without problem:
|
@TingLei-NOAA @wangym1111 Thanks, |
@hu5970 @TingLei-NOAA @wangym1111 The previous regression test on case #3 is because of a submulde issue that uses fix files in master branch.
1/1 Test #3: [=[rrfs_3denvar_glbens]=] ........ Passed 739.48 sec 100% tests passed, 0 tests failed out of 1 Total Test time (real) = 739.50 sec |
Four questions:
|
To the first question, I think the "approval" after all those conversations could be used to show all conversations have finished to the reviewers' satisfactions. |
@hu5970 and @hongli-wang could you please go over this PR and click "Resolve conversation" button if the comments or questions from reviewers are addressed. |
Thank you, @TingLei-NOAA , for your reply. Would you mind resolving your open conversations? |
@RussTreadon-NOAA as we talked on this before. For reviewers without write permission to EMC GSI, they could not "resolve" those conversations by any "resolve" button. Of course, I could reply at the end of each conversation of mine with "resolved" . But for the number of those conversations, that is really an unnecessary burden.. So, I hope the final "approval " by a certain reviewer could avoid any concerns to unresolved conversation related to the reviewer. Thanks. |
Thank you @TingLei-NOAA. My apologies for forgetting our previous conversation. You are right. You can not resolve conversations even if you initiate the conversation.
The PR author, in this case @hongli-wang , should resolve conversations after consultation with peer reviewers. If the PR author can not resolve conversations, a member of the handling review team must do so. |
@RussTreadon-NOAA Thanks a lot for sorting it out! |
@RussTreadon-NOAA Regarding reviews, I had addressed all the comments. I am working on resolving these conversions now. Thanks. |
Thank you @hongli-wang for confirming that the code in the PR is functionally correct with regards to FED EnVar assimilation. Thank you also for resolving open conversations. |
@TingLei-NOAA @RussTreadon-NOAA @hu5970 Thanks, |
Thank you @hongli-wang for confirming that regression tests for this PR have not yet been run on Orion. GSI developers use Orion. If you have an account on Orion, I encourage you to run ctests on Orion as well as Hera. I am in the process of cloning |
I did a regression test on Orion:
The "netcdf_fv3_regional" failed because of a little over the memory limitation. It is not a fatal failure. |
On the 4th question, we are still testing SDL/VDL. After we finalize the configuration of the RRFS GSI, we will make several cases for RRFS. |
Thank you @hu5970 for running regression tests for this PR on Orion. Your ctest timings are consistent with behavior reported in g-w issue #1996. |
Sounds good. Any idea how many new tests several means? We just reduced the number of ctests to 7. It would be nice to keep the number of ctests relatively low. |
@RussTreadon-NOAA One case for GSI hybrid analysis (if we use SDL/VDL). Two cases for EnKF (conventional and radar dbz analysis). But we will remove current rrfs and netcdf_fv3_regional. So, we will have 8 cases. |
Great! Add 3, remove 2. Thanks for letting us know about future changes in the GSI regression test suite. |
Description
Please include relevant motivation and context
** Please include a summary of the change and which issue is fixed**
Please provide reference to the issue this pull request is addressing
Please see
Fixes #622
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist
DUE DATE for this PR is 11/10/2023. If this PR is not merged into
develop
by this date, the PR will be closed and returned to the developer.