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

Update operational workflow to work with GSI develop branch #3068

Draft
wants to merge 29 commits into
base: release/gfs.v16.4
Choose a base branch
from

Conversation

ADCollard
Copy link
Contributor

Update operational workflow to work with GSI develop branch. This mostly involves changing the links to elements that have been moved out of the GSI repository since GFSv16.3. The ultimate aim is to be able to cycle with v16 workflow but using a version of GSI close to develop. This will not reproduce operations.

Resolves #3045

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? YES
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? YES
    • GSI

How has this been tested?

Testing is limited to WCOSS2. Initialised full resolution cycling experiment. Will compare to operational performance.

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary

@ADCollard ADCollard added the production update Processing update in production label Nov 5, 2024
@ADCollard ADCollard marked this pull request as draft November 5, 2024 17:04
@emilyhcliu
Copy link
Contributor

@ADCollard I see that most file changes are due to the changes in the .gitignore.
These changes make sense. I see that the CRTM is still crtm 2.4.1.
I am checking out the branch to build.
Two questions.

  • Will this global-workflow branch build GSI from the GSI develop?
  • Do you want me to run tests with this global-workflow branch?

@ADCollard
Copy link
Contributor Author

@ADCollard I see that most file changes are due to the changes in the .gitignore. These changes make sense. I see that the CRTM is still crtm 2.4.1. I am checking out the branch to build. Two questions.

  • Will this global-workflow branch build GSI from the GSI develop?
  • Do you want me to run tests with this global-workflow branch?

@emilyhcliu Yes, the idea is that this is a v16 workflow that will checkout the develop version of GSI (eventually it will be a specific tag).

I am mostly looking for feedback on whether this is the right way to do this. I have also started a real time parallel on WCOSS2 (this will only run on WCOSS2), but right now the machine is unavailable, so it might be easiest just to look at those results.

Yes, the CRTM version is 2.4.0.1 which is the current operational version. I don't think we have time to move to CRTM v3.0 for GFSv16.4, but that would be addressed via a different issue anyway.

Thanks for looking at this!

Copy link
Member

@KateFriedman-NOAA KateFriedman-NOAA left a comment

Choose a reason for hiding this comment

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

We can't bring in most v17 script updates and need to stick closer to the v16 versions for this update. Please see my comments and let me know if I can assist at all. Thanks @ADCollard !

jobs/JGDAS_ATMOS_VERFOZN Outdated Show resolved Hide resolved
jobs/JGDAS_ATMOS_VERFRAD Outdated Show resolved Hide resolved
jobs/JGDAS_ATMOS_VMINMON Outdated Show resolved Hide resolved
jobs/JGFS_ATMOS_VMINMON Outdated Show resolved Hide resolved
@@ -0,0 +1,91 @@
#! /usr/bin/env bash

source "$HOMEgfs/ush/preamble.sh"
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above. Can't use preamble but thankfully it looks like this script hasn't changed much between v16 and v17 so you can likely keep most of the v17 version:

[kfriedma@hercules-login-2 scripts]$ pwd
/work/noaa/global/kfriedma/git_hercules/dev-gfs.v16.GSIdev/scripts
[kfriedma@hercules-login-2 scripts]$ diff ../../release-gfs.v16.3.21/scripts/exgdas_atmos_verfozn.sh exgdas_atmos_verfozn.sh
1c1
< #/bin/sh
---
> #! /usr/bin/env bash
3c3
< set -ax
---
> source "$HOMEgfs/ush/preamble.sh"
12,13d11
< export scr=exgdas_vrfyozn.sh
<
73c71
<    count=`ls diag* | grep ".nc4" | wc -l`
---
>    count=$(ls diag* | grep ".nc4" | wc -l)
76,77c74,75
<       for filenc4 in `ls diag*nc4.gz`; do
<          file=`echo $filenc4 | cut -d'.' -f1-2`.gz
---
>       for filenc4 in $(ls diag*nc4.gz); do
>          file=$(echo $filenc4 | cut -d'.' -f1-2).gz
92,98d89
<
< if [[ "$VERBOSE" = "YES" ]]; then
<    echo "end exgdas_vrfyozn.sh, exit value = ${err}"
< fi
<
<
< set +x

Comment on lines +94 to +110
#-----------------------------------
#--add gfs_wafs link if checked out
if [ -d ${pwd}/gfs_wafs.fd ]; then
#-----------------------------------
cd ${pwd}/../jobs ||exit 8
$LINK ../sorc/gfs_wafs.fd/jobs/* .
cd ${pwd}/../parm ||exit 8
[[ -d wafs ]] && rm -rf wafs
$LINK ../sorc/gfs_wafs.fd/parm/wafs wafs
cd ${pwd}/../scripts ||exit 8
$LINK ../sorc/gfs_wafs.fd/scripts/* .
cd ${pwd}/../ush ||exit 8
$LINK ../sorc/gfs_wafs.fd/ush/* .
cd ${pwd}/../fix ||exit 8
[[ -d wafs ]] && rm -rf wafs
$LINK ../sorc/gfs_wafs.fd/fix/* .
fi
Copy link
Member

Choose a reason for hiding this comment

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

This WAFS block shouldn't be getting added. Need to recheck the sync with the release/gfs.v16.3.21 branch and/or remove this block in your branch. The release/gfs.v16.4 branch will need to get synced with the prior release branches again anyway but let's remove this block now.

Comment on lines +116 to +120
# The following are now explicitly in the global-workflow repository
# $LINK ../sorc/gsi.fd/ush/gsi_utils.py .
# $LINK ../sorc/gsi.fd/ush/calcanl_gfs.py .
# $LINK ../sorc/gsi.fd/ush/calcinc_gfs.py .
# $LINK ../sorc/gsi.fd/ush/getncdimlen .
Copy link
Member

Choose a reason for hiding this comment

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

Good to know, let's remove these lines now.

@@ -0,0 +1,115 @@
#! /usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this script from your branch. This won't go into the GFS with v16 and still needs sign off from NCO for v17.

ush/preamble.sh Outdated
@@ -0,0 +1,92 @@
#! /usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this from your branch. This won't go into the GFS with v16 and still needs sign off from NCO for v17.

ush/rstprod.sh Outdated
@@ -0,0 +1,19 @@
#! /usr/bin/env bash

source "$HOMEgfs/ush/preamble.sh"
Copy link
Member

Choose a reason for hiding this comment

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

If we definitely need this script from the v17 package then remove the preamble line.

@ADCollard
Copy link
Contributor Author

We can't bring in most v17 script updates and need to stick closer to the v16 versions for this update. Please see my comments and let me know if I can assist at all. Thanks @ADCollard !

@KateFriedman-NOAA Thank you for this review. This is exactly the type of feedback I was looking for.

Just so I am clear on this, why are the preamble.sh and jjob_header.sh scripts forbidden in v16? I will attempt to make these scripts look closer to v17, but I want to be sure if there is something particularly problematic with these.

@KateFriedman-NOAA
Copy link
Member

Just so I am clear on this, why are the preamble.sh and jjob_header.sh scripts forbidden in v16? I will attempt to make these scripts look closer to v17, but I want to be sure if there is something particularly problematic with these.

@ADCollard The preamble and jjob_header scripts are new as of v17 and we have not yet had them signed off on by NCO. They represent a big change to how we structure the JJOB scripts so we need them approved by NCO. Both scripts are also intended to be in every JJOB/scripts script so we shouldn't partially introduce them.

@ADCollard
Copy link
Contributor Author

Just so I am clear on this, why are the preamble.sh and jjob_header.sh scripts forbidden in v16? I will attempt to make these scripts look closer to v17, but I want to be sure if there is something particularly problematic with these.

@ADCollard The preamble and jjob_header scripts are new as of v17 and we have not yet had them signed off on by NCO. They represent a big change to how we structure the JJOB scripts so we need them approved by NCO. Both scripts are also intended to be in every JJOB/scripts script so we shouldn't partially introduce them.

@KateFriedman-NOAA OK. I will remove them.

Copy link
Member

@KateFriedman-NOAA KateFriedman-NOAA left a comment

Choose a reason for hiding this comment

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

Looks good, left one change to make. Can rereview when moved out of draft. Thanks!

Correct prepobs  version
@ADCollard
Copy link
Contributor Author

Looks good, left one change to make. Can rereview when moved out of draft. Thanks!

@KateFriedman-NOAA Thanks for looking at this. Before I take this out of draft I need to test it in a cycled experiment. I am on foreign travel right now and hence cannot access WCOSS-2. I will run the test on my return on 11/26/2024.

I am also inviting @EdwardSafford-NOAA to look over what I have done with the monitoring suite.

@KateFriedman-NOAA
Copy link
Member

Sounds good, thanks @ADCollard ! FYI I will be on leave the week of Thanksgiving.

@EdwardSafford-NOAA
Copy link
Contributor

@ADCollard Monitoring changes look ok, though I think you might need to have radmon_err_rpt.sh in the ush directory. If you point me to your output and/or logs from this branch I can confirm or deny that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
production update Processing update in production
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants