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

HAFSv1 needed HYCOM, UPP, g2tmpl, ugwp, and moving nesting related updates #1544

Merged
merged 29 commits into from
Jan 17, 2023

Conversation

BinLiu-NOAA
Copy link
Contributor

@BinLiu-NOAA BinLiu-NOAA commented Dec 20, 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.

Instructions: All subsequent sections of text should be filled in as appropriate.

The information provided below allows the code managers to understand the changes relevant to this PR, whether those changes are in the ufs-weather-model repository or in a subcomponent repository. Ufs-weather-model code managers will use the information provided to add any applicable labels, assign reviewers and place it in the Commit Queue. Once the PR is in the Commit Queue, it is the PR owner's responsibility to keep the PR up-to-date with the develop branch of ufs-weather-model.

Description

Provide a detailed description of what this PR does. What bug does it fix, or what feature does it add? Is a change of answers expected from this PR? Are any library updates included in this PR (modulefiles etc.)?

HAFSv1 needed updates and changes, including:

  • Update submodule FV3/UPP to point its latest develop branch and update postxconfig-NT-hafs.txt.
  • Switch to use g2tmpl/1.10.2.
  • Switch to use the unified ugwp scheme for HAFS CCPP suites and regression tests.
  • Update submodule HYCOM and enable debug and check all build for the HYCOM component (from @danrosen25 and @BijuThomas-NOAA).
  • Fix the artificial gravity wave issue caused by HAFS moving nest leading edge crossing steep topography by enable using same coarse grid topography in the moving nest leading edge (from @wramstrom).
  • Bug fix in calculation of moving-nest halo weights identified by @BijuThomas-NOAA in DDEBUG=ON builds (from @wramstrom).

Notes:

  • Will change baseline results so new baseline will need to be generated.
  • New input data under FV3_hafs_input_data need to be added/populated.
    Please use and populate the following FV3_hafs_input_data to replace its counterpart:
    /scratch1/NCEPDEV/hwrf/save/Bin.Liu/RT/NEMSfv3gfs/input-data-20221101/FV3_hafs_input_data
    This FV3_hafs_input_data dir is backward compatible, since there are only new files added. No changes for existing files. In this case, no need to update INPUT data version unless that is preferred.

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues must always be created before starting work on a PR branch!)

Testing

Regression tests on WCOSS2 went through successfully. Baseline changes are expected.

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
  • opnReqTest for newly added/changed feature
  • CI

Dependencies

If testing this branch requires non-default branches in other repositories, list them. Those branches should have matching names (ideally).

Do PRs in upstream repositories need to be merged first?
If so add the "waiting for other repos" label and list the upstream PRs

@BinLiu-NOAA BinLiu-NOAA marked this pull request as ready for review December 21, 2022 22:30
@jkbk2004
Copy link
Collaborator

@BinLiu-NOAA we are working on #1523. It's like to merge in #1523 tomorrow morning. Then we can open up our schedule for this pr, if you are working tomorrow.

@BinLiu-NOAA
Copy link
Contributor Author

Thanks, @jkbk2004! For HAFSv1, we are still waiting for some potential UPP develop branch updates. Also, this PR could be combined with another WW3 submodule update (also needed by HAFSv1). With that, I am setting this PR as draft PR for now.

@jkbk2004
Copy link
Collaborator

sounds good!

@BinLiu-NOAA BinLiu-NOAA changed the title Update submodule FV3/UPP and switch to use g2tmpl/1.10.2 HAFSv1 needed UPP, g2tmpl, ugwp updates Dec 30, 2022
@BinLiu-NOAA BinLiu-NOAA changed the title HAFSv1 needed UPP, g2tmpl, ugwp updates HAFSv1 needed WW3, HYCOM, UPP, g2tmpl, ugwp, and moving nesting related updates Dec 30, 2022
@BinLiu-NOAA BinLiu-NOAA marked this pull request as ready for review January 6, 2023 14:13
@jkbk2004
Copy link
Collaborator

jkbk2004 commented Jan 6, 2023

@BinLiu-NOAA can we check if it's ok to schedule this pr to commit on Monday? or if you like to merge by Monday morning, then we can make a decision to work from today.

@BinLiu-NOAA
Copy link
Contributor Author

BinLiu-NOAA commented Jan 6, 2023

Thanks, @jkbk2004! We are still waiting 1-2 changes from WW3 and/or UPP for this PR. So, if possible, can we try to target this PR merge saying next Wednesday (01/11)? Please also make sure the PR #1554 is ahead of this PR if possible. Thanks!

@BinLiu-NOAA BinLiu-NOAA changed the title HAFSv1 needed WW3, HYCOM, UPP, g2tmpl, ugwp, and moving nesting related updates HAFSv1 needed HYCOM, UPP, g2tmpl, ugwp, and moving nesting related updates Jan 11, 2023
Copy link
Collaborator

@DeniseWorthen DeniseWorthen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would have been better to include one or more debug tests for HAFS since the issue w/ HYCOM is now fixed w/ this update. I don't think we have any other moving-nest applications so that feature should be tested in the HAFS application.

@BrianCurtis-NOAA
Copy link
Collaborator

@jkbk2004 Why was -DCCPP_SUITES added to the debug tests?

We noticed compile 012 has random build issue. The problem is clearly noticed on jet. The build issue behaves way better with specifically adding-DCCPP_SUITES options that we need on regression test. With this change, no impact on result.

@DusanJovic-NOAA if i recall correctly, we removed CCPP_SUITES in order to test a build with all suites, get rid of a few extra builds as well. Were there any other reasons we removed the CCPP_SUITES option for the debug builds?

@jkbk2004 If there are build issues with a compile of all suites, then I think at the very least an issue should be made about this so we can return a build with all suites.

@DusanJovic-NOAA
Copy link
Collaborator

@DusanJovic-NOAA if i recall correctly, we removed CCPP_SUITES in order to test a build with all suites, get rid of a few extra builds as well. Were there any other reasons we removed the CCPP_SUITES option for the debug builds?

Correct. However, currently we have 96 suites. but only about a third is used/tested. There was discussion about removing all unnecessary suites. See: NOAA-EMC/fv3atm#510

@BinLiu-NOAA
Copy link
Contributor Author

I think it would have been better to include one or more debug tests for HAFS since the issue w/ HYCOM is now fixed w/ this update. I don't think we have any other moving-nest applications so that feature should be tested in the HAFS application.

@DeniseWorthen, yes, we plan to add a debug HAFS RT in a future PR. Thanks!

@jkbk2004
Copy link
Collaborator

@jkbk2004 Why was -DCCPP_SUITES added to the debug tests?

We noticed compile 012 has random build issue. The problem is clearly noticed on jet. The build issue behaves way better with specifically adding-DCCPP_SUITES options that we need on regression test. With this change, no impact on result.

@DusanJovic-NOAA if i recall correctly, we removed CCPP_SUITES in order to test a build with all suites, get rid of a few extra builds as well. Were there any other reasons we removed the CCPP_SUITES option for the debug builds?

@jkbk2004 If there are build issues with a compile of all suites, then I think at the very least an issue should be made about this so we can return a build with all suites.

Yes, we can create an issue to extend discussion to #1450

@jkbk2004
Copy link
Collaborator

created a new issue #1568

@BrianCurtis-NOAA
Copy link
Collaborator

I've made some changes that only effect WCOSS2 systems. It moves to a newer ecflow version on those systems and forces the use of their ecflow login nodes. This is to resolve an issue where their script ecfrunjob was hanging and causing 100% cpu usage and memory leakage on the regular login nodes.

@BrianCurtis-NOAA
Copy link
Collaborator

@jkbk2004 Ready for upstream merges.

@jkbk2004
Copy link
Collaborator

@BinLiu-NOAA can you update submodule pointers and revert branches in gitmodules? so we can finish merging.

@jkbk2004 jkbk2004 self-requested a review January 17, 2023 20:58
@BinLiu-NOAA
Copy link
Contributor Author

@jkbk2004 The submodules should be up to date now. Thanks!

@BinLiu-NOAA
Copy link
Contributor Author

@jkbk2004, BTW, once this PR is merged, could you please help to branch out a production/hafs.v1 branch from the develop (which will be used for HAFSv1 code freeze). Feel free to let me know if that is not recommended. If so, we will just create and host this production/hafs.v1 branch inside the hafs-community fork instead. Thanks!

@DeniseWorthen
Copy link
Collaborator

@BinLiu-NOAA @junwang-noaa I believe the place to create the tag would be here on UWM rather than hafs-community. Jun, is that right?

@BinLiu-NOAA
Copy link
Contributor Author

@BinLiu-NOAA @junwang-noaa I believe the place to create the tag would be here on UWM rather than hafs-community. Jun, is that right?

@DeniseWorthen, that would also be what we would prefer to, branching out inside ufs-weather-model official repo (saying production/hafs.v1) and then later on creating a tag for code delivery and implementation. Thanks!

@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Jan 17, 2023

That sounds good. I referred to a tag, but you actually want a new branch, production/hafs.v1.

@BinLiu-NOAA
Copy link
Contributor Author

That sounds good. I referred to a tag, but you actually want a new branch, production/hafs.v1.

@DeniseWorthen, at this moment, for HAFSv1 code freeze, we would like to have a branch (production/hafs.v1). There may or may not be further changes needed before we delivery the HAFSv1 package to NCO later on (in several weeks). And when we deliver the code to NCO, we can create a tag from this production/hafs.v1 branch then. Thanks!

@jkbk2004 jkbk2004 merged commit e051e0e into ufs-community:develop Jan 17, 2023
@DeniseWorthen
Copy link
Collaborator

@BinLiu-NOAA production/hafs.v1 branch has been created. Good luck!

@jkbk2004
Copy link
Collaborator

@BinLiu-NOAA production/hafs.v1 branch has been created. Good luck!

Thanks @DeniseWorthen for following up with the branch!

@BinLiu-NOAA
Copy link
Contributor Author

Thanks, @jkbk2004 and @DeniseWorthen!

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 New Input Data Req'd This PR requires new data to be sync across platforms Waiting for Reviews The PR is waiting for reviews from associated component PR's.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update submodule FV3/upp for HAFSv1
7 participants