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 soil temperature and moisture IAU #2415

Open
wants to merge 51 commits into
base: develop
Choose a base branch
from

Conversation

tsga
Copy link

@tsga tsga commented Aug 27, 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:

Add Incremental Analysis Update (IAU) to soil temperature and moisture in the NoahMP land model of the UWM's CCPP.

Commit Message:

* UFSWM 
  * FV3
    * ccpp-physics - Add Incremental Analysis Update (IAU) capability to update soil temperature and moisture for CCPP's NoahMP land model

Priority:

  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

UFSWM Blocking Dependencies:

  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • PR Adds New Tests/Baselines.
    Additional land-IAU specific test to be added: control_c48_intel_land_iau (based on the "control_c48_intel" test, with the only difference being the turning on of land IAU and soil temperature increment files inside INPUT directory). The test inputs have been staged in hera platform. The baseline outputs created for the control_c48_lnd_iau_intel regression tests is at:
    /scratch1/NCEPDEV/stmp4/Tseganeh.Gichamo/REGRESSION_TEST_LAND_IAU/control_c48_lnd_iau_intel

Input data Changes:

  • New input data.
    soil temperature increment files inside INPUT directory

Library Changes/Upgrades:

  • No Updates

Testing Log:

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

@tsga
Copy link
Author

tsga commented Sep 16, 2024

Tagging relevant people here. Please add (tag) anyone you think should be aware.
@CoryMartin-NOAA, @barlage, @ClaraDraper-NOAA, @CatherineThomas-NOAA, @junwang-noaa

@tsga tsga reopened this Sep 16, 2024
@tsga tsga marked this pull request as ready for review September 16, 2024 12:48
@grantfirl
Copy link
Collaborator

@tsga There should be an associated PR for the NoahMP changes in https://github.com/NOAA-EMC/noahmp, right? This component repo needs to have its internal NoahMP code updated to correspond to the changes in ufs-community/ccpp-physics#222. Please let me know if you need help with this or if I am mistaken.

@grantfirl
Copy link
Collaborator

@tsga Since there is a new namelist to read in (according to https://github.com/ufs-community/ccpp-physics/pull/222/files#diff-13b994b587cb8a841246d6db7df55ef9ff6fc1269777b9fe1b2e704ce9da629bR138), will an example of this be added to this repository for testing purposes?

@tsga
Copy link
Author

tsga commented Sep 23, 2024

@tsga Since there is a new namelist to read in (according to https://github.com/ufs-community/ccpp-physics/pull/222/files#diff-13b994b587cb8a841246d6db7df55ef9ff6fc1269777b9fe1b2e704ce9da629bR138), will an example of this be added to this repository for testing purposes?

@grantfirl yes there is an example namelist with the new Land IAU specific test at:
/scratch1/NCEPDEV/da/Tseganeh.Gichamo/ccpp-physics/test/control_c48_intel_land_iau/input.nml

Should I also add it as part of this repo (as one of the files)?

@grantfirl
Copy link
Collaborator

@tsga Since there is a new namelist to read in (according to https://github.com/ufs-community/ccpp-physics/pull/222/files#diff-13b994b587cb8a841246d6db7df55ef9ff6fc1269777b9fe1b2e704ce9da629bR138), will an example of this be added to this repository for testing purposes?

@grantfirl yes there is an example namelist with the new Land IAU specific test at: /scratch1/NCEPDEV/da/Tseganeh.Gichamo/ccpp-physics/test/control_c48_intel_land_iau/input.nml

Should I also add it as part of this repo (as one of the files)?

@tsga @jkbk2004 As of now, if we merged your work, there would be no tests that are utilizing it, so it could be broken with subsequent code changes. You mention in this PR description that a new RT should be added, but the files haven't been modified to add it yet. You'd need to edit tests/rt.conf, commit the new namelist and refer to it in the new test file to be added in tests/tests, and have the new data staged on the testing platforms in order for the test to be set up to be exercised.

@grantfirl
Copy link
Collaborator

@tsga Please see tsga#1,
tsga/fv3atm#1, and tsga/ccpp-physics#1

With these changes, I can use the new RT via: ./rt.sh -e -k -n "control_c48_lnd_iau intel" > rt_single_control_c48_lnd_iau.log 2>&1 &

It compiles successfully, although the run fails because the data is not staged. When the data is staged, we can complete testing.

@tsga
Copy link
Author

tsga commented Oct 12, 2024

@grantfirl Thank you! I added the increment files to the INPUT dir of the test you set up and it runs successfully (I am working on the other issues of gracefully exiting when there are no files, etc, you mentioned earlier).
In the mean time, can you copy these files (sfc_inc*.nc) /scratch2/NCEPDEV/land/data/DA/soil_moisture/LandIAU/INC/C48/INC09/*
to /scratch2/NAGAPE/epic/UFS-WM_RT/NEMSfv3gfs/input-data-20240501/FV3_input_data/INPUT_L127_gfsv17 (I don't have permission). These six files are the only ones needed to make the control_c48_lnd_iau intel test that you set up complete.

@grantfirl
Copy link
Collaborator

@grantfirl Thank you! I added the increment files to the INPUT dir of the test you set up and it runs successfully (I am working on the other issues of gracefully exiting when there are no files, etc, you mentioned earlier). In the mean time, can you copy these files (sfc_inc*.nc) /scratch2/NCEPDEV/land/data/DA/soil_moisture/LandIAU/INC/C48/INC09/* to /scratch2/NAGAPE/epic/UFS-WM_RT/NEMSfv3gfs/input-data-20240501/FV3_input_data/INPUT_L127_gfsv17 (I don't have permission). These six files are the only ones needed to make the control_c48_lnd_iau intel test that you set up complete.

@tsga I don't have permissions in that directory either. @jkbk2004 and/or other members of the UFS code management team can do this for us.

@jkbk2004
Copy link
Collaborator

@grantfirl Thank you! I added the increment files to the INPUT dir of the test you set up and it runs successfully (I am working on the other issues of gracefully exiting when there are no files, etc, you mentioned earlier). In the mean time, can you copy these files (sfc_inc*.nc) /scratch2/NCEPDEV/land/data/DA/soil_moisture/LandIAU/INC/C48/INC09/* to /scratch2/NAGAPE/epic/UFS-WM_RT/NEMSfv3gfs/input-data-20240501/FV3_input_data/INPUT_L127_gfsv17 (I don't have permission). These six files are the only ones needed to make the control_c48_lnd_iau intel test that you set up complete.

@tsga I don't have permissions in that directory either. @jkbk2004 and/or other members of the UFS code management team can do this for us.

@tsga @grantfirl I will follow up today. will keep you posted.

@jkbk2004
Copy link
Collaborator

@tsga /scratch2/NAGAPE/epic/UFS-WM_RT/NEMSfv3gfs/input-data-20240501/FV3_input_data/INPUT_L127_gfsv17 is for C96 resolution. Do you want me to copy over /scratch2/NCEPDEV/land/data/DA/soil_moisture/LandIAU/INC/C96/sfc_inc.tile*.nc ?

@jkbk2004
Copy link
Collaborator

@tsga Or if you want to stay with with C48, we have C48 test case at /scratch2/NAGAPE/epic/UFS-WM_RT/NEMSfv3gfs/input-data-20240501/FV3_input_data48/INPUT_L127_gfsv17. We can copy over your C48 sfc increment files there. Maybe both C48 and C96. Let me know.

@tsga
Copy link
Author

tsga commented Oct 16, 2024

@tsga Or if you want to stay with with C48, we have C48 test case at /scratch2/NAGAPE/epic/UFS-WM_RT/NEMSfv3gfs/input-data-20240501/FV3_input_data48/INPUT_L127_gfsv17. We can copy over your C48 sfc increment files there. Maybe both C48 and C96. Let me know.

@jkbk2004 yes, let's stay with C48. And disregard the earlier directory. The C48 increments are at: /scratch2/NCEPDEV/land/data/DA/soil_moisture/LandIAU/INC/C48/INC3

@jkbk2004
Copy link
Collaborator

@tsga /scratch2/NAGAPE/epic/UFS-WM_RT/NEMSfv3gfs/input-data-20240501/FV3_input_data48/INPUT_L127_gfsv17/sfc_inc.tile*.nc is available. Once test is ok on hera, then we can rsync up on other machines.

@tsga
Copy link
Author

tsga commented Nov 16, 2024

@jkbk2004 Thanks for fixing the git submodules and syncing up.
I have re-run the tests with the latest changes. Everything looks good and the sub-repos are cleaned up.
The baseline created for the control_c48_lnd_iau_intel regression tests is at:
/scratch1/NCEPDEV/stmp4/Tseganeh.Gichamo/REGRESSION_TEST_LAND_IAU/control_c48_lnd_iau_intel

@jkbk2004 jkbk2004 added New Baselines New baselines will be added to project. Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. labels Nov 16, 2024
Copy link
Collaborator

@BrianCurtis-NOAA BrianCurtis-NOAA left a comment

Choose a reason for hiding this comment

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

I'm confused here. There is a new test that creates new baselines, yet there's no entries in test_changes.list. This should have been taken care of before testing started.

NOAHMP-interface/CMakeLists.txt Outdated Show resolved Hide resolved
@jkbk2004
Copy link
Collaborator

I'm confused here. There is a new test that creates new baselines, yet there's no entries in test_changes.list. This should have been taken care of before testing started.

@BrianCurtis-NOAA this pr adds new test case control_c48_lnd_iau intel. I pushed test_changes.list

@BrianCurtis-NOAA
Copy link
Collaborator

I'm confused here. There is a new test that creates new baselines, yet there's no entries in test_changes.list. This should have been taken care of before testing started.

@BrianCurtis-NOAA this pr adds new test case control_c48_lnd_iau intel. I pushed test_changes.list

@jkbk2004 It looks like there was a full suite tested on hera: d93cf7a but the new test never made it into that log, which might be why the test_changes.list was empty. Make sure every commit is reviewed in some capacity to help ensure everything is good to go prior to starting testing.

NOAHMP-interface/CMakeLists.txt Outdated Show resolved Hide resolved
@BrianCurtis-NOAA
Copy link
Collaborator

Skip Acorn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Baselines New baselines will be added to project. 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 Land IAU Capability to NoahMP Driver in CCPP
8 participants