Develop ugwp stoch phys fix #1696
Conversation
…en and affecting stochastic physics in ugwp
|
@jkbk2004 According to @yangfanglin , this PR needs to be the top priority. I see that you have #1677 and #1669 in the queue already, but this one needs to go first, I think. It is a simple bugfix with no RT changes. @mdtoyNOAA There is an inconsistency in the description for this PR. The checklist item that says RT results change is checked, but it is mentioned in the Testing section that no changes are expected to baselines. Please correct this to avoid confusion. |
|
The PR could be merged with #1677 |
Thanks @grantfirl. I unchecked the "RT's have changed" box. |
|
@mdtoyNOAA #1681 was merged in. Can you resolve the conflict? You may save your hera intel log and just attach to PR. If attached in PR, it might be an easier process to resolve the conflict by not directly pushing to branch. @lisa-bengtsson one question is about HR1 tag. It sounds like the tag will be created after this PR. Then, my question is the code change from #1677 will be ok with the HR1 tag? @grantfirl @yangfanglin @Qingfu-Liu Can we confirm? @lisa-bengtsson Even if we need to separately merge in #1677, we still have time this week. |
|
Since PR #1677 is also a bug fix, it may be of interest to the ensemble team to use it ,as it will be part of HR2 (for GFSv17). Therefore it could be good to merge the two. It should be straightforward. If a new HR1b tag is going to be created, it would be good to understand what exactly is the difference to HR1. |
|
Sure! @lisa-bengtsson it sounds like it will be easier way to combine ufs-community/ccpp-physics#61 to your ufs-community/ccpp-physics#57. Would that be ok for you to pull in those two fixes in drag_suite files? And then we can start working on #1677. |
@mdtoyNOAA @grantfirl @dustinswales @Qingfu-Liu is this OK with you, or we do it in two separate steps and create the HR1b/EP4 tag after? |
|
@BrianCurtis-NOAA @DeniseWorthen we are combing into #1677. |
Yep, totally fine to combine this into #1677 |
…r-model into develop_ugwp_stoch_phys_fix
@jkbk2004 I pulled the latest change and pushed to my repo. How do I "attach" my hera intel log to the PR? |
|
@mdtoyNOAA let's go to #1677. @lisa-bengtsson combined in drag_suite code changes. Once you approve it, we can start testing there. I will move there. |
|
Merged with #1677 |
PR Checklist
This PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.
This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR
An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
are specified below.
Results for one or more of the regression tests change and the reasons for the changes are understood and explained below.
New or updated input data is required by this PR. If checked, please work with the code managers to update input data sets on all platforms.
Description
This PR fixes a bug that occurs when running stochastic physics. As part of ccpp-physics PR#22, the order in which subroutines "gwdps_run" and "drag_suite_run" were called by subroutine "unified_ugwp_run" was reversed. Subroutine "drag_suite_run" is called last, but the variable "RDXZB" (the level of the dividing streamline for blocking) is initialized, which zeros out the value that was calculated in "gwdps_run". RDXZB is needed outside of the unified_ugwp subroutine by stochastic physics. This caused stochastic physics runs to crash. The bug was not caught by regression testing.
The problem has been fixed by changing the declaration of "RDXZB" from "intent(out)" to "intent(inout)" in drag_suite.F90 and drag_suite.meta, and initializing it in drag_suite.F90 only when the GSL drag suite blocking is performed, i.e., when "do_gsl_drag_ls_bl" is equal to .true.
*** This fix needs to be done for running the GEFS.v13 reanalysis and reforecasts.
Issue(s) addressed
This PR fixes Issue #62.
Testing
The stochastic physics team (@pjpegion et al) has tested the revised code, and the crashes no longer occur. Regression tests and ORT's have passed (Hera -- Intel) with no changes to the baseline.
Dependencies
If testing this branch requires non-default branches in other repositories, list them. Those branches should have matching names (ideally).
https://github.com/mdtoyNOAA/fv3atm: develop_ugwp_stoch_phys_fix branch.
Do PRs in upstream repositories need to be merged first?
If so add the "waiting for other repos" label and list the upstream PRs
CCPP PR#61.
NOAA-EMC/fv3atm PR#642