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

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

Merged
merged 46 commits into from
Mar 27, 2024

Conversation

SamuelTrahanNOAA
Copy link
Collaborator

@SamuelTrahanNOAA SamuelTrahanNOAA commented Mar 21, 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:

Part 1: atparse.bash

The tests/atparse.bash ignored the last line of the input if it lacked an end-of-line character (see #2199).

Now it will process such a line, and insert an end-of-line at the end if one was missing. (Some regression tests assume the output ends with an end-of-line.)

Also, I commented the file and updated the test program.

Part 2: Regression test system Rocoto fixes

A stray <partition></partition> on Hera rocoto_workflow.xml caused sporadic job failures on the new Hera Rocky nodes (#2206)

Inconsistencies in Rocoto logging have been corrected (see #2207)

And there has been some general clean-up in the tests/ directory, such as only loading rocoto or ecflow modules when needed and replacing tabs with eight spaces.

Parts 3 & 4: Combined two other pull requests to this one.

Pull requests #2185 and #2197 have been merged into this branch. Neither should change results. Preliminary testing on Hera and Hercules has shown no changes to baselines from this combined PR.

Commit Message:

* 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

Priority:

  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

UFS-WM PR 2185
FV3: sub-PR 799
ccpp-physics: sub-PR 184

UFS-WM PR 2197
MOM6 sub-PR 130

UFSWM Blocking Dependencies:

  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • No Baseline Changes.

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)

@SamuelTrahanNOAA
Copy link
Collaborator Author

I'm running regression tests on Hercules now. Will report back when I have something interesting to say about them.

@BrianCurtis-NOAA
Copy link
Collaborator

@SamuelTrahanNOAA If you get some time, could you cleanup that script with: https://www.shellcheck.net/ i have work moving towards a GH action to check bash scripts, and am working towards updating all scripts in UFSWM to comply.

I ask because I am not sure I 100% understand this file, which might come in handy to comply with shellcheck, and it seems you might.

If not, that's fine, it will be worked on either way.

@SamuelTrahanNOAA
Copy link
Collaborator Author

I ask because I am not sure I 100% understand this file

Hopefully the comments I'm adding will help people understand it.

@SamuelTrahanNOAA
Copy link
Collaborator Author

I ran it through ShellCheck and found only one genuine error. ShellCheck's other complaints were limitations of its parser.

@BrianCurtis-NOAA
Copy link
Collaborator

I ran it through ShellCheck and found only one genuine error. ShellCheck's other complaints were limitations of its parser.

Thanks!

@SamuelTrahanNOAA
Copy link
Collaborator Author

Some of the regression tests assume atparse always outputs a trailing end-of-line character. I'll update it to add the missing end-of-line.

@SamuelTrahanNOAA SamuelTrahanNOAA marked this pull request as ready for review March 21, 2024 20:39
@SamuelTrahanNOAA
Copy link
Collaborator Author

And now it works!

All tests passed on Hercules with no changes to baselines.

@jkbk2004
Copy link
Collaborator

@SamuelTrahanNOAA I always wonder why we use @[] for parsing parm namlist files. I mean why not ${}. Parsing with environment might be a handy approach to parse with simple bash script. Any idea?

@SamuelTrahanNOAA
Copy link
Collaborator Author

I always wonder why we use @[] for parsing parm namlist files. I mean why not ${}. Parsing with environment might be a handy approach to parse with simple bash script. Any idea?

Using ${varname} prevents you from parsing a shell script with atparse. The syntax must be easily identifiable from any other language it may parse. No languages that I know of use @[varname], so that's what I went with.

@SamuelTrahanNOAA
Copy link
Collaborator Author

The full version of atparse is more capable. You can do things like @[varname:-default value], there are if/else-if/else/end-if equivalents, and you can compare variables. The bash version only has @[varname].

@BrianCurtis-NOAA
Copy link
Collaborator

Also curious, why are we using atparse? What was the need it met?

@jkbk2004
Copy link
Collaborator

It appears @lisa-bengtsson is on leave until Friday, so it is unlikely we'll hear back from her. The commit message has a sentence straight from her PR description, so the content should be correct. I think we can use it even if we don't hear back.

Yes, @lisa-bengtsson OK'd to merge once all tests run successfully. The commit messages look good.

@SamuelTrahanNOAA
Copy link
Collaborator Author

I see two problems left:

The FV3 PR has not been reviewed: NOAA-EMC/fv3atm#799. If there are problems, someone will need to edit the branch to fix them.

Once the CCPP PR is merged (ufs-community/ccpp-physics#184) someone will need to update .gitmodules in the branch for the FV3 PR (NOAA-EMC/fv3atm#799) before the FV3 PR can be merged.

@FernandoAndrade-NOAA
Copy link
Collaborator

FYI Gaea is currently very unresponsive today to the point that it is taking a long time to clone and does so incorrectly. I've opened up a support ticket with them to see what's going on.

@jkbk2004
Copy link
Collaborator

No baseline change! we can skip Gaea due to the system issue. We can follow up with Gaea in the next PR if there is issue.

@SamuelTrahanNOAA
Copy link
Collaborator Author

We are changing the rt.sh slightly, and it is possible that change will break GAEA. Unlikely, but possible. Consider running only a few of the regression tests on GAEA when it comes back to see if the regression test system works.

@BrianCurtis-NOAA
Copy link
Collaborator

Acorn needed an adjustment to the changes where the ecflow if statements were, i'll push them after I run testing. Give me time to get those tests running.

@BrianCurtis-NOAA
Copy link
Collaborator

There's still something not quite right with Acorn using ecflow. The problem i'm having happens with both the current dev branch and this branch, so it's not a UFSWM issue, yet, at least. It's not worth holding this PR up. Please skip Acorn.

@zach1221
Copy link
Collaborator

There's still something not quite right with Acorn using ecflow. The problem i'm having happens with both the current dev branch and this branch, so it's not a UFSWM issue, yet, at least. It's not worth holding this PR up. Please skip Acorn.

Ok, I think we're skipping Gaea as well, due to continued latency. So, we can begin the final review/merging process.

@zach1221
Copy link
Collaborator

@SamuelTrahanNOAA Fv3atm and MOM6 PRs are merged.
NOAA-EMC/fv3atm@1ba8410
NOAA-EMC/MOM6@ab7bd14

@zach1221 zach1221 merged commit c54e986 into ufs-community:develop Mar 27, 2024
zhanglikate added a commit to zhanglikate/ufs-weather-model that referenced this pull request May 3, 2024
commit f234a3e
Author: Ufuk Turunçoğlu <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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) <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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) <[email protected]>
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
No Baseline Change No Baseline Change Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
None yet
8 participants