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

Add omega calculation and change LSM diag option to RTs #2373

Merged
merged 17 commits into from
Aug 1, 2024

Conversation

dpsarmie
Copy link
Collaborator

@dpsarmie dpsarmie commented Jul 19, 2024

Commit Queue Requirements:

  • Fill out all sections of this template.
  • All sub component pull requests have been reviewed by their code managers.
  • Run the full Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
  • Commit 'test_changes.list' from previous step

Description:

This PR adds the new omega calculation for RTs that are run in non-hydrostatic mode. Changes have been made to diag_tables and only the global_control.nml.IN. The original implementation of the omega calculation in FV3 can be found here: #2327 and the related global workflow PR that implements this is here: NOAA-EMC/global-workflow#2751 .

IOPT_DIAG has been set to 2 for the remaining coupled test cases that had it set to 1. This should be the new default moving forward with the merge of #2354. Related GW issue is NOAA-EMC/global-workflow#2786.

Commit Message:

* UFSWM - Adds pass_full_omega_to_physics_in_non_hydrostatic_model parameter in the global_control.nml.IN namelist and
          sets it to true for non-hydrostatic runs. An additional field (omga) has been added to the diag_tables.

Priority:

  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

  • None

UFSWM Blocking Dependencies:

  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • PR Updates/Changes Baselines.

Input data Changes:

  • None.

Library Changes/Upgrades:

  • No Updates

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • Gaea
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
  • opnReqTest (complete task if unnecessary)

@dpsarmie dpsarmie added enhancement New feature or request Baseline Updates Current baselines will be updated. labels Jul 19, 2024
@XiaqiongZhou-NOAA
Copy link
Contributor

XiaqiongZhou-NOAA commented Jul 22, 2024

Thank you very much for taking care of this
In terms of the diag_table used for regional and nesting, you do not need add omga since it will keep original way - calculate omega in post. No baseline should be changed for all regional and HAFS's regression tests.

@dpsarmie
Copy link
Collaborator Author

Thank you very much for taking care of this In terms of the diag_table used for regional and nesting, you do not need add omga since it will keep original way - calculate omega in post. No baseline should be changed for all regional and HAFS's regression tests.

Ok, thank you for taking a look at this. I will add those changes later today.

@dpsarmie dpsarmie closed this Jul 22, 2024
@dpsarmie dpsarmie force-pushed the feature/update_omega_calc branch from 6995bc0 to c127601 Compare July 22, 2024 17:08
This PR adds the omega calculation to select diag_tables and
the default_vars.sh script. The older version of this branch
had calculations apply to regional tests. This version reverts
those changes and now the omega update is only applied to global
RTs.
@dpsarmie dpsarmie reopened this Jul 23, 2024
@dpsarmie
Copy link
Collaborator Author

I went ahead and redid the PR. The only changes now are to the global RTs and the regional RTs will remain as is.

@dpsarmie dpsarmie self-assigned this Jul 23, 2024
@dpsarmie dpsarmie added the Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. label Jul 23, 2024
junwang-noaa
junwang-noaa previously approved these changes Jul 23, 2024
Reset test_changes.list for LSM updates that will be added.
@dpsarmie dpsarmie dismissed stale reviews from junwang-noaa and BrianCurtis-NOAA via d70ce2d July 24, 2024 17:53
dpsarmie and others added 2 commits July 24, 2024 13:53
With LSM upgrades in PR ufs-community#2354, the IOPT_DIAG option will
now be set to 2 instead of 1. This change was in most
RTs. This commit will change the option for the remaining
RTs that did not have this option.
@dpsarmie
Copy link
Collaborator Author

More changes were rolled into this PR. A couple of the RTs were missing the LSM option that should be a part of #2354. Those have been added and tested.

@HelinWei-NOAA thanks for pointing out the issue and let me know if anything else needs to be rolled in here.

I'll edit the overall PR text to reflect this soon.

@FernandoAndrade-NOAA
Copy link
Collaborator

Please go ahead and resync your branch, we'd like to get started with testing this PR today, thank you.

@FernandoAndrade-NOAA
Copy link
Collaborator

Please also commit the updated test_changes.list to verify which baselines need updates.

@dpsarmie
Copy link
Collaborator Author

Please also commit the updated test_changes.list to verify which baselines need updates.

I had to regenerate the list, I'll ping you here once it's committed.

@dpsarmie
Copy link
Collaborator Author

@FernandoAndrade-NOAA Ok test_changes.list was added. Let me know if you need anything else.

@FernandoAndrade-NOAA
Copy link
Collaborator

@BrianCurtis-NOAA getting started with baseline creation for this PR.

@FernandoAndrade-NOAA
Copy link
Collaborator

If manually editing test_changes.list in the future, please be sure to verify the editor is using LF instead of CRLF, otherwise it'll insert hidden characters that will cause issues

@BrianCurtis-NOAA
Copy link
Collaborator

Before testing can start it will need a new bl-date

@zach1221 zach1221 added jenkins-ort run ORT testing and removed jenkins-ort run ORT testing labels Jul 31, 2024
@FernandoAndrade-NOAA
Copy link
Collaborator

Adding a note that the following tests failed comparison on Gaea after successful baseline recreation:
control_wam_debug intel
gnv1_nested intel

@BrianCurtis-NOAA
Copy link
Collaborator

Adding a note that the following tests failed comparison on Gaea after successful baseline recreation: control_wam_debug intel gnv1_nested intel

neither of those show up in the test_changes.list and the wam_debug one failed on WCOSS2. regenerate those baselines and try those two again. @dpsarmie can you check your old run logs to see if those tests passed?

@dpsarmie
Copy link
Collaborator Author

Adding a note that the following tests failed comparison on Gaea after successful baseline recreation: control_wam_debug intel gnv1_nested intel

neither of those show up in the test_changes.list and the wam_debug one failed on WCOSS2. regenerate those baselines and try those two again. @dpsarmie can you check your old run logs to see if those tests passed?

Just checked. The run that generated the test_changes.list was on Hercules and those tests are turned off for the Hercules platform. I'll run the tests on Hera and report back soon.

@BrianCurtis-NOAA
Copy link
Collaborator

Adding a note that the following tests failed comparison on Gaea after successful baseline recreation: control_wam_debug intel gnv1_nested intel

neither of those show up in the test_changes.list and the wam_debug one failed on WCOSS2. regenerate those baselines and try those two again. @dpsarmie can you check your old run logs to see if those tests passed?

Just checked. The run that generated the test_changes.list was on Hercules and those tests are turned off for the Hercules platform. I'll run the tests on Hera and report back soon.

@jkbk2004 This is a good data point to emphasize that we should work towards all tests being run on one platform or devise a testing method where all tests are run between two machines prior to joining the commit queue.

@dpsarmie
Copy link
Collaborator Author

Ok, full suite has run on Hera and yes, control_wam_debug intel and gnv1_nested intel are showing as needing new baselines.

@zach1221
Copy link
Collaborator

zach1221 commented Aug 1, 2024

Ok, testing is complete, we can proceed with final review.

@zach1221 zach1221 merged commit ee4f19a into ufs-community:develop Aug 1, 2024
3 checks passed
WalterKolczynski-NOAA pushed a commit to NOAA-EMC/global-workflow that referenced this pull request Aug 23, 2024
Add a parameter "pass_full_omega_to_physics_in_non_hydrostatic_mode" in
FV3 namelist. It was set to "true" to use a new method to diagnose
omega. This PR is based on the /ufs-community/ufs-weather-model#2327)

Corresponding parameter changed in GFSv17 related regression tests
ufs-community/ufs-weather-model#2373))
@dpsarmie dpsarmie deleted the feature/update_omega_calc branch August 27, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated. enhancement New feature or request Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add updated omega calculation pass to ufs-wm
8 participants