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

Fix for land component model #2191

Merged
merged 48 commits into from
Apr 30, 2024

Conversation

uturuncoglu
Copy link
Collaborator

@uturuncoglu uturuncoglu commented Mar 14, 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 aims to fix couple of issues with the component land model. The fully coupled atm+lnd configuration (control_p8_atmlnd) was failing in debug mode. To fix the issue, the NOAHMP NUOPC cap is modified to read required fixed information from the sfc_data file. The control_p8_atmlnd and control_p8_atmlnd_sbs tests are started to use V2 surface files which seems consistent. control_p8_atmlnd_debug test is also included to the test list.

Commit Message:

* UFSWM - fix fully coupled land component configuration
  * NOAHMP - get fixed information from surface file

Priority:

  • Critical Bugfix: control_p8_atmlnd configuration was not running correctly
  • High: The masking was wrong in CMEPS and lnd->atm direction of coupling was also not working properly for control_p8_atmlnd. The input files was also not consistent and as a result baseline was also wrong.

Git Tracking

UFSWM:

Sub component Pull Requests:

UFSWM Blocking Dependencies:


Changes

Regression Test Changes (Please commit test_changes.list):

  • PR Adds New Tests/Baselines. control_p8_atmlnd_debug
  • PR Updates/Changes Baselines. control_p8_atmlnd, control_p8_atmlnd_sbs and control_restart_p8_atmlnd

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)

@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 @DeniseWorthen I am making this ready for review and depend on #2175. This will fix additional issues revealed by testing #2175.

@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 @DeniseWorthen BTW, I am not sure it is already available or not but it would be nice to have capability in RT testing framework to allow using exiting tests to create new ones by changing couple of options in it. So, it would be something like an overloading. For example, this PR introduces another test for debug and the only change in the test file is the baseline directory since it is complied with debug mode. The rest of the options are same. So, it would be nice to use a set of base tests and use them to create new ones. I think this could simply the the tests files and also their consistencies.

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Apr 1, 2024

@jkbk2004 @DeniseWorthen BTW, I am not sure it is already available or not but it would be nice to have capability in RT testing framework to allow using exiting tests to create new ones by changing couple of options in it. So, it would be something like an overloading. For example, this PR introduces another test for debug and the only change in the test file is the baseline directory since it is complied with debug mode. The rest of the options are same. So, it would be nice to use a set of base tests and use them to create new ones. I think this could simply the the tests files and also their consistencies.

@uturuncoglu Right now, debug test is handled as separate test case with debug build option on it. This pr is exactly doing that. However, I fully got your point. On EPIC side, we consider to revamp the test system to introduce rt.yaml (#2182). Goal is to allow more flexibility and even to meet the different requirements of projects across centers and labs. Hope to make progress soon with the yaml approach (in a month time scale): https://github.com/jkbk2004/ufs-weather-model/blob/feature/rt-refactor/tests/rt.yaml. Once a sharable feature e branch is available, I will keep you posted there.

@DusanJovic-NOAA
Copy link
Collaborator

DusanJovic-NOAA commented Apr 2, 2024

@uturuncoglu If I understand correctly what you are asking, you can already do that relatively easily. In the derived test configuration file simply source the base test configuration file and then overwrite variables you need to have different values.
Suppose you have a base test named 'base_test' which has the test configuration file in tests/base_test and it probably has the content something like:

$ cat tests/base_test

export TEST_DESCR="Base test to test specific configuration"
export CNTL_DIR=base_test
export LIST_FILES="sfcf000.nc \
                   sfcf021.nc \
                   sfcf024.nc \
                   atmf000.nc \
                   atmf021.nc \
                   atmf024.nc"
export_fv3
export VAR1=.true.
export VAR2=365
export VAR3='enable_feature'

Now, let's say you want to run a debug version of this test. Create file tests/base_test_debug, for example like:

$ cat tests/base_test_debug

# "Inherit" all settings from 'base_test'
source tests/base_test

# Overwrite some configuration variables
export TEST_DESCR="Base test to test specific configuration - DEBUG version"
export CNTL_DIR=base_test_debug
export LIST_FILES="sfcf000.nc \
                   sfcf001.nc \
                   atmf000.nc \
                   atmf001.nc"
export VAR1=.false.

You can keep 'deriving' more tests from base_test_debug, by sourcing it in a third file, etc. Or another test that derives from the base_test. You can build a family (tree) of closely related tests.

There's nothing special or magic about these files, they are just shell files sourced from run_test.sh script to set test specific variables. That's all. You can run any shell command in these files or whatever you need to do to set variables required by that specific test.

@uturuncoglu
Copy link
Collaborator Author

@DusanJovic-NOAA Thanks. That is what I am looking for. I'll update the test with this approach. It makes the life easier.
@jkbk2004 Thanks for the information. It is great to know that testing system will be improved.

@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 I synced the component and model in my fix branch. I wonder if this could go into commit queue soon since it has some critical fix for land component model. I could also try to introduce a fully coupled configuration in this PR by using cpld_control_p8 as a reference per @junwang-noaa request.

@jkbk2004
Copy link
Collaborator

@jkbk2004 I synced the component and model in my fix branch. I wonder if this could go into commit queue soon since it has some critical fix for land component model. I could also try to introduce a fully coupled configuration in this PR by using cpld_control_p8 as a reference per @junwang-noaa request.

@uturuncoglu sounds good! we may be able to schedule to commit around Friday. I will keep you posted.

@DeniseWorthen DeniseWorthen added Baseline Updates Current baselines will be updated. New Baselines New baselines will be added to project. labels Apr 16, 2024
@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 Thank you. Best,

@DeniseWorthen
Copy link
Collaborator

@uturuncoglu Are you able to merge develop and run the test suite on Hercules or Orion and then commit the test_changes.list?

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen Okay. I synced. I'll run the full RTs and update you.

@DeniseWorthen
Copy link
Collaborator

Great. I'd like to get this committed, since it has been waiting.

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen Thanks for your help. I am running RTs on Hercules now.

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen I commit test_changes.list. It looks fine to me and only fully coupled configurations (incl. side-by-side) are failing due to answer change and also debug test is failing since it has no baseline yet.

@DeniseWorthen
Copy link
Collaborator

@uturuncoglu Yes, that's perfect. If you can get Mike or someone to review the NoahMP PR, this can be marked as ready.

@zach1221
Copy link
Collaborator

Testing complete.

@zach1221
Copy link
Collaborator

@uturuncoglu please update noahmp submodule hash and revert .gitmodule url.
hash: NOAA-EMC/noahmp@1e25901

jkbk2004
jkbk2004 previously approved these changes Apr 30, 2024
@uturuncoglu
Copy link
Collaborator Author

Sorry all. There is a sickness situation at home at this point. I am back. So, let me know if you need anything from my side.

@zach1221
Copy link
Collaborator

@BrianCurtis-NOAA @jkbk2004 I've appended the bit_base test to the regional_control_gnu ORT log. You'll have to re-review the PR following the change.

@uturuncoglu
Copy link
Collaborator Author

@zach1221 @jkbk2004 @BrianCurtis-NOAA I think there is a minor bug in rt.sh on head of develop. See,

DISKNM=/"work/noaa/epic/UFS-WM_RT"
Do you want to add fix to this PR?

@jkbk2004
Copy link
Collaborator

@zach1221 @jkbk2004 @BrianCurtis-NOAA I think there is a minor bug in rt.sh on head of develop. See,

DISKNM=/"work/noaa/epic/UFS-WM_RT"

Do you want to add fix to this PR?

fixed

@zach1221 zach1221 self-requested a review April 30, 2024 17:26
@zach1221 zach1221 merged commit f234a3e into ufs-community:develop Apr 30, 2024
3 checks passed
zhanglikate added a commit to zhanglikate/ufs-weather-model that referenced this pull request May 3, 2024
commit f234a3e
Author: Ufuk Turunçoğlu <turuncu@ucar.edu>
Date:   Tue Apr 30 11:35:25 2024 -0600

    Fix for land component model (ufs-community#2191)

    * UFSWM - fix fully coupled land component configuration
      * NOAHMP - get fixed information from surface file

commit 04bbc15
Author: jiandewang <jiande.wang@noaa.gov>
Date:   Thu Apr 25 14:52:00 2024 -0400

    update MOM6 to its main repo. 20240401 commit (ufs-community#2241)

    * UFSWM -
      * MOM6 - update MOM6 to its main repo. 20240401 commit (NCAR-candidate-20240319)

commit b6c576d
Author: Daniel Sarmiento <42810219+dpsarmie@users.noreply.github.com>
Date:   Tue Apr 23 12:24:22 2024 -0400

    Merged global namelist (ufs-community#2173)

    * UFSWM - global_control.nml_IN has been added as the new regression test namelist template for all global regression tests. The namelist now uses pointers (i.e. @[abc]) for variables and default values have been added to the default_vars.sh script. A new section in default_vars.sh has been added (export_tiled) to account for tiled RTs that pulls the correct parameter files using the ATMRES variable.
    Regression tests have been modified to account for these changes. Tests that were not compatible with the GFSv17_p8 core have been disabled for now. They will be turned on as they are updated from GFSv16 to GFSv17.

commit 5d2ca19
Author: WenMeng-NOAA <48260754+WenMeng-NOAA@users.noreply.github.com>
Date:   Fri Apr 19 13:59:12 2024 -0400

    Update upp submodule (ufs-community#2213)

    * UFSWM - Update inline post
      * FV3 - Update upp submodule for inline post

commit 47c0099
Author: Brian Curtis <64433609+BrianCurtis-NOAA@users.noreply.github.com>
Date:   Wed Apr 17 15:59:48 2024 -0400

    Add bash linting to CI. Cleanup .sh scripts a bit. Address .sh bugs. Adds -v Verbose option. (ufs-community#2218)  Remove nowarn Intel compiler flag (ufs-community#2225)

    * UFSWM
    - Add bash linting to CI:
      - uses superlinter to check for consistent bash code writing
    - Cleans up .sh scripts to comply with superlinter
    - Cleans up .sh scripts to be more consistent, easier to read.
    - Add's -v verbose option if debugging outputs needed, otherwise simplifies rt.sh run echo's.
    - Addresses smaller bugs
      - quota/timeout search logic adjusted.
      - check for dirs existing (DISKNM, STMP, PTMP) before starting.
      - adjustments/cleanup to ecflow/rocoto sections
      - rt.sh will attempt to start ecflow, and only stop ecflow if it started from rt.sh.
      - fix for issue where run_dir will not delete properly.
    * FV3: Address compiler warnings
      * atmos_cubed_sphere: Address compiler warnings.

commit 4f32a4b
Author: Rick Grubin <152905742+rickgrubin-tomorrow@users.noreply.github.com>
Date:   Mon Apr 15 07:21:08 2024 -0600

    Document ATMW / ATMAERO / HAFS WM configurations (ufs-community#2160)

    * UFSWM
      * doc/Userguide
        * source
          * conf.py
          * Configurations.rst
          * FAQ.rst
          * InputsOutputs.rst
          * Introduction.rst

commit ac4445d
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Apr 15 08:59:42 2024 -0400

    Bump idna from 3.6 to 3.7 in /doc/UsersGuide (ufs-community#2234)

    *doc/UserGuide
       *requirements.txt - updates inda version from 3.6 to 3.7

commit 281b32f
Author: Samuel Trahan (NOAA contractor) <39415369+SamuelTrahanNOAA@users.noreply.github.com>
Date:   Mon Apr 15 08:38:01 2024 -0400

    bug fixes: kchunk3d ignored, hailwat uninitialized in dycore, tile_num wrong for nests (ufs-community#2201)

    * UFSWM - None.
      * FV3 - Write component will use kchunk3d. Model init sends the right tile number to CCPP.
        * atmos_cubed_sphere - Initialize the hailwat variable. Pass global_tile index to model.

commit 8a5f711
Author: Denise Worthen <denise.worthen@noaa.gov>
Date:   Thu Apr 11 13:32:26 2024 -0400

    Add PIO namelist control for CICE (ufs-community#2145)

    Update to CICE-Consortium/CICE aca8357. Adds implementation of namelist PIO options for CICE

commit 45c8b2a
Author: JONG KIM <jong.kim@noaa.gov>
Date:   Thu Apr 4 19:49:13 2024 -0400

    Hotfix/cubed sphere hash fix: HAILCAST diagnostic code (units issue) (ufs-community#2223)

    cubed_sphere hash update: f060e85 for a bug- fix in the HAILCAST diagnostic code (units issue)

commit 26e6db6
Author: Denise Worthen <denise.worthen@noaa.gov>
Date:   Wed Apr 3 19:57:08 2024 -0400

    Enable cpl_scalars export from ATM and NoahMP for use by CMEPS (ufs-community#2175)

      * CMEPS - allow additional dimension in cpl_scalars for CSG and regional ATM domains for use in mediator history files
      * CMEPS - fix mapping mask for lnd->atm
      * FV3 - add export of cpl_scalars
      * NOAHMP - add export of cpl_scalars

commit 1411b90
Author: Dusan Jovic <48258889+DusanJovic-NOAA@users.noreply.github.com>
Date:   Mon Apr 1 18:04:44 2024 -0400

    Update module_write_netcdf to avoid hangs in RRFS runs (ufs-community#2193)

    * UFSWM - Update module_write_netcdf to avoid hangs in RRFS runs
      * FV3 - Update module_write_netcdf to avoid hangs in RRFS runs

commit 87c27b9
Author: Matthew Masarik <86749872+MatthewMasarik-NOAA@users.noreply.github.com>
Date:   Fri Mar 29 15:23:42 2024 -0400

    WW3 feature:  Langmuir turbulence parameterization (ufs-community#2195)

      * WW3 - Langmuir turbulence parameterization

commit c54e986
Author: Samuel Trahan (NOAA contractor) <39415369+SamuelTrahanNOAA@users.noreply.github.com>
Date:   Wed Mar 27 16:11:03 2024 -0400

    regression test system bug fixes, eliminate MOM6 warnings (ufs-community#2197), add xr_cnvcld flag to FV3 (ufs-community#2185) (ufs-community#2202)

    * UFSWM - atparse.bash: correctly handle input that doesn't end with an end-of-line character. Fix some bugs in Rocoto support and clean up rt.sh.
      * FV3 - namelist flag xr_cnvcld to control if suspended grid-mean convective cloud condensate should be included in cloud fraction and optical depth calculation in radiation in the GFS suite
        * ccpp - physics-level changes to implement new namelist variable
      * MOM6 - update MOM6 code to eliminate all compiler warnings
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. 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.

Land component model configurations that has active atmosphere component are not working in debug mode
9 participants