Skip to content

Update Regression Test to include gfsv17 prototypes that are after prototype 8, and turn on prognostic closure.#1480

Merged
jkbk2004 merged 74 commits into
ufs-community:developfrom
lisa-bengtsson:progc_update
Dec 5, 2022
Merged

Update Regression Test to include gfsv17 prototypes that are after prototype 8, and turn on prognostic closure.#1480
jkbk2004 merged 74 commits into
ufs-community:developfrom
lisa-bengtsson:progc_update

Conversation

@lisa-bengtsson
Copy link
Copy Markdown
Contributor

@lisa-bengtsson lisa-bengtsson commented Oct 28, 2022

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

These PR’s in ufs-weather-model, fv3atm and ccpp/physics does the following:
Creates a set of _gfsv17 RT’s to reflect tests that are beyond prototype 8 targeted for GFSv17
Activates prognostic closure by setting namelist progsigma = true in *gfsv17 and pointing to new field_table in the RT’s
Addresses additional parametric tuning in progclosure_calc.F90
Adds a new gfsv17 field_table and diag_table

The ufs-weather-model PR was also merged with a PR including outerloop for the wave model, and the ccpp/physics PR was merged with a fix for ccpp_prebuild.

Issue(s) addressed

#1477

Testing

How were these changes tested? What compilers / HPCs was it tested with? Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) Have regression tests and unit tests (utests) been run? On which platforms and with which compilers? (Note that unit tests can only be run on tier-1 platforms)

  • hera.intel
  • hera.gnu
  • orion.intel
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss2.intel
  • acorn.intel (skip for queue upgrade testing)
  • opnReqTest for newly added/changed feature
  • CI

Dependencies

ufs-community/ccpp-physics#18
NOAA-EMC/ufsatm#598

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

@ChunxiZhang-NOAA @junwang-noaa @yangfanglin @DeniseWorthen @JessicaMeixner-NOAA can you take a close look at these tests and provide feedback. I'm working on the CCPP PR to go with this PR as well, it should be merged together. New baselines would be needed for the new tests. Thank you!

@lisa-bengtsson lisa-bengtsson changed the title Progc update Update Regression Test to include gfsv17 prototypes that are after prototype 8, and turn on prognostic closure. Oct 28, 2022
Comment thread tests/tests/control_gfsv17 Outdated
@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

I added export DNATS=0 in the new gfsv17 tests in order to not exclude any tracers from advection since the default value is DNATS=2, I wonder if this (DNATS = 0) should also be the case in the test cpld_control_noaero_p8?

@ChunxiZhang-NOAA
Copy link
Copy Markdown
Contributor

cpld_control_noaero_p8

Good catch. DNATS should be 0 for cpld_control_noaero_p8, otherwise TKE will not be advected.

@junwang-noaa
Copy link
Copy Markdown
Collaborator

junwang-noaa commented Oct 31, 2022

@lisa-bengtsson @ChunxiZhang-NOAA @JessicaMeixner-NOAA SInce you are adding the GFSv17 tests, would you please remove the control related tests from GFSv16? Otherwise the RT is taking too long to finish for code commit.

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

cpld_control_noaero_p8

Good catch. DNATS should be 0 for cpld_control_noaero_p8, otherwise TKE will not be advected.

Ok, I will update those noaero tests.

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

@lisa-bengtsson @ChunxiZhang-NOAA @JessicaMeixner-NOAA SInce you are adding the GFSv17 tests, would you please remove the control related tests from GFSv16? Otherwise the RT is taking too long to finish for code commit.

@junwang-noaa can you list which control tests can be removed? So far I'm only adding cpld_control ORT tests, and one uncoupled control_gfsv17. Thanks.

@junwang-noaa
Copy link
Copy Markdown
Collaborator

This is good question for GFSv17 test owners. My understanding is that all the control related test except control_p8 related tests

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

@junwang-noaa Who are the code owners for the GFSv16 tests? Perhaps what should be done is to remove the GFSv16 tests, and create ORT tests based on control_gfsv17 instead? Although this doesn't help in removing any tests... It is not clear to me what can be removed. Thanks.

@junwang-noaa
Copy link
Copy Markdown
Collaborator

@lisa-bengtsson We can't run GFSv16 configuration with the control related test any more. Yes, we need to remove all these tests and create ORT tests based on control_gfsv17. Any new feature gfsv17 test needs to be based on control_gfsv17.

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

lisa-bengtsson commented Nov 1, 2022

@lisa-bengtsson We can't run GFSv16 configuration with the control related test any more. Yes, we need to remove all these tests and create ORT tests based on control_gfsv17. Any new feature gfsv17 test needs to be based on control_gfsv17.

@junwang-noaa Note that replacing the previous gfsv16 tests with gfsv17 tests will not reduce the total tests from this PR as we are adding additional cpld_...gfsv17 tests and not removing existing cpld...p8 tests. This is because it was suggested that those should be replaced by cpld_...gefsv13 tests in the future. Your suggestion sounds like it is something that the test owners and code managers need to do, I would propose in a separate PR.

@DeniseWorthen
Copy link
Copy Markdown
Collaborator

DeniseWorthen commented Nov 1, 2022

@lisa-bengtsson You've added new nml.IN files for gfsv17. Other than the progsima variable, these are identical to the existing p8 versions. Do you need to add the new nml files? Or, do you envision that the future gefsv13 (currently p8) would eventually diverge too much from what is the gfsv17 for that to make sense.

Also, I believe you will also need to add a bmark_restart_gfsv17 test.

Finally, there's a problem w/ your .gitmodules, which points to a non-existent FV3 fork (lisa.bengtsson).

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

@lisa-bengtsson You've added new nml.IN files for gfsv17. Other than the progsima variable, these are identical to the existing p8 versions. Do you need to add the new nml files? Or, do you envision that the future gefsv13 (currently p8) would eventually diverge too much from what is the gfsv17 for that to make sense.

Also, I believe you will also need to add a bmark_restart_gfsv17 test.

Finally, there's a problem w/ your .gitmodules, which points to a non-existent FV3 fork (lisa.bengtsson).

Hi Denise, thanks I updated the .gitmodules.

My understanding is that the final code base and configuration of gefsv13 is not decided yet, but will have to be finalized within the next 2 months. gfsv17 will continue to be developed until the early summer 2023, so in the end they will diverge. @yangfanglin can you and/or Avichal weigh in on the strategy for these tests?

@jkbk2004
Copy link
Copy Markdown
Collaborator

jkbk2004 commented Dec 5, 2022

We will skip jenkins-ci. We can start merging process.

@ChunxiZhang-NOAA
Copy link
Copy Markdown
Contributor

@jkbk2004 ccpp-physics PR was merged.

@jkbk2004
Copy link
Copy Markdown
Collaborator

jkbk2004 commented Dec 5, 2022

@jkbk2004 ccpp-physics PR was merged.

thanks!

Comment thread .gitmodules Outdated
@BrianCurtis-NOAA
Copy link
Copy Markdown
Collaborator

FV3 hash is NOAA-EMC/ufsatm@92b0386

@BrianCurtis-NOAA
Copy link
Copy Markdown
Collaborator

@lisa-bengtsson FV3 hash isn't updated yet. Should be 92b0386, but is currently bd39951

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

@lisa-bengtsson FV3 hash isn't updated yet. Should be 92b0386, but is currently bd39951

ok, let me try again.

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

@lisa-bengtsson FV3 hash isn't updated yet. Should be 92b0386, but is currently bd39951

ok, let me try again.

hm, I'm confused, should I not to "git pull upstream develop"? It doesn't seem to point to the correct one if I do that?

@BrianCurtis-NOAA
Copy link
Copy Markdown
Collaborator

@lisa-bengtsson FV3 hash isn't updated yet. Should be 92b0386, but is currently bd39951

ok, let me try again.

hm, I'm confused, should I not to "git pull upstream develop"? It doesn't seem to point to the correct one if I do that?

You should do a git remote update then git checkout upstream/develop then it should be on the correct hash. You can check with git submodule status, each submodules hash will show.

@BrianCurtis-NOAA
Copy link
Copy Markdown
Collaborator

I gave one wrong instruction. in FV3 you would do the git remote update then git checkout upstream/develop at that point it should be the hash you're looking for, then in UFSWM you can do a git submodule status to see the FV3 hash, then git add FV3 will get it into UFSWM.

@BrianCurtis-NOAA
Copy link
Copy Markdown
Collaborator

Looks good now. Thanks!

@lisa-bengtsson
Copy link
Copy Markdown
Contributor Author

@BrianCurtis-NOAA Ok, thank you for your help!

@jkbk2004 jkbk2004 merged commit 659bd98 into ufs-community:develop Dec 5, 2022
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants