Skip to content
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

Ufs/dev ugwp stoch phys fix #61

Merged

Conversation

mdtoyNOAA
Copy link
Collaborator

@mdtoyNOAA mdtoyNOAA commented Apr 5, 2023

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.

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.

This PR fixes Issue #62.

This fix needs to be done for running the GEFS.v13 reanalysis and reforecasts.

@yangfanglin
Copy link
Collaborator

@grantfirl @dustinswales @Qingfu-Liu Please consider giving this PR the highest priority for committing. It is needed for running the Ensemble Protype-4 for GEFS.v13 development.

@junwang-noaa A new model tag, for instance, GFSv17.HR1b, needs to be created after this PR is committed to the latest develop branch. -- Thanks.

@junwang-noaa
Copy link

@yangfanglin Do you mean we will create a new cut HR1 tag from the latest develop branch after this PR is committed? This will introduce 2 months of model code updates since Jan 30 when the previous HR1 tag was created. Also global workflow is using current HR1 tag, it may break before the code changes in the last two months are brought to global workflow.

@Qingfu-Liu
Copy link
Collaborator

Qingfu-Liu commented Apr 5, 2023 via email

@lisa-bengtsson
Copy link
Collaborator

I don't think we should make a model tag HR1b including the two months of model code changes. Why not include it in the ensemble prototype runs, and have it as an update for HR2?

@yangfanglin
Copy link
Collaborator

@junwang-noaa I do not think there are any recent model changes that will break the ensemble forecast. But you have a good point about the workflow. My understanding is that the team is not, and will not use the official workflow for conducting EP4 and reforecast experiments. WalterKolczynski-NOAA and @bingfu-NOAA can you confirm ?

@yangfanglin
Copy link
Collaborator

yangfanglin commented Apr 5, 2023

There are two options. (1) commit this PR to the development branch since it is a bug fix, create a new tag HR1b for the ensemble team. (2) commit this bug fix to both the development branch and update the HR1 tag.

My preference is (1) since there are some recent infrastructure related changes that can potentially improve computational performance.

For example, using the newly added capability of "Write restart files using the write grid component" may lead to reduced CPU and node cost for conducting the massive reforecast experiments.

@bingfu-NOAA
Copy link

@yangfanglin @junwang-noaa
Current global-workflow can not be used to run ensemble experiments, all the ensemble prototypes used modified global-worklow (adding specific ensemble settings and stochastic settings) for experiments.

@Qingfu-Liu
Copy link
Collaborator

Qingfu-Liu commented Apr 5, 2023 via email

@junwang-noaa
Copy link

@DeniseWorthen FYI.

@DeniseWorthen
Copy link

@Qingfu-Liu I will create a new HR1b tag when PR #61 is merged. It will be created from the current develop branch.

@bingfu-NOAA
Copy link

@junwang-noaa @yangfanglin I think option (1) as suggested by Fanglin is also preferable to ensemble experiments.

@lisa-bengtsson
Copy link
Collaborator

@bingfu-NOAA PR #57 is a bug fix as well that you may want to include in your prototype runs if they have not started yet. It should be merged next in line.

@yangfanglin maybe we should carefully look at all the updates in all the model components between HR1 and HR1b? Do we know if there is anything that requires namelist updates in the ensemble workflow in the other component models? There were quite many PR's merged in the last 2 months.

@bingfu-NOAA
Copy link

@lisa-bengtsson Thank you for pointing it out. Yes, ensemble prototype 4 has not started yet. I think we can use that bug fix once it is committed.

@lisa-bengtsson
Copy link
Collaborator

@lisa-bengtsson HR1 tag was created after this commit Bug fix for cloud effective radius for convective clouds (HR1) ([https://github.com/ufs-community/ufs-weather-model/pull/1582)

One can find at https://github.com/ufs-community/ufs-weather-model/commits/develop all commits after the above PR.

Thank you @yangfanglin I will take a look. I don't really know exactly which PR's from waves, ice and ocean that will actually be used in our prototype runs from this list, but we could maybe bring it up in the next meeting on prototypes.

@junwang-noaa
Copy link

@lisa-bengtsson @yangfanglin @bingfu-NOAA I don't see updates from ocean, ice, wave that will change model results except PR#1692, which is expected to improve the atm-wave coupling. But we do have the restart filename updates for atm/ocn/ice/wave, which may require corresponding updates. The rest are physics updates. @DeniseWorthen FYI.

@pjpegion
Copy link
Collaborator

pjpegion commented Apr 5, 2023

How about tagging this as EP4?

@lisa-bengtsson
Copy link
Collaborator

@junwang-noaa thank you! It looks like no other physics updates besides #57 and #61 will impact GEFS/GFS.

@grantfirl grantfirl merged commit 94eaf51 into ufs-community:ufs/dev Apr 6, 2023
@DeniseWorthen
Copy link

@yangfanglin Was there a final decision on what to tag the new UFS hash once this is fully merged? I see a suggestion of EP4 instead of HR1b.

@yangfanglin
Copy link
Collaborator

@DeniseWorthen Yes, EP4 is the favorite by consensus.

@DeniseWorthen
Copy link

Tag EP4 has been created.

@dustinswales
Copy link
Collaborator

Corresponding EP4 tag created in ccpp-physics

@bingfu-NOAA
Copy link

Thank you all for helping on making the PR and creating the tag in a short time.

grantfirl pushed a commit that referenced this pull request Jul 18, 2023
update main with the develop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix issue with level of dividing streamline (rdxzb) being overwritten and affecting stochastic physics in ugwp
10 participants