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

Release/gfsda.v16.3.0 (prune_4nco_globa.sh) #466

Conversation

RussTreadon-NOAA
Copy link
Contributor

The PR corrects errors in the prune function of ush/prune_4nco_global.sh. Please see issue #465 for details.

Both the prune and restore functions have been tested in g-w release/gfs.v16.3.0 using sorc/build_gsi.sh with the export PRUNE_4NCO="YES" line activated. The prune function removes from the working copy of sorc/gsi.fd those files and directories which are not used by GFS v16.3.0. The restore function restores pruned files and directories from the repository.

Neither the prune nor the restore functions alter GFS v16 DA analysis results.

@MichaelLueken
Copy link
Contributor

Hi @RussTreadon-NOAA. Do we want the functionality of ush/prune_4nco_global.sh on all machines, or just WCOSS2?

I ask because the version of git dictates the command required to restore the deleted contents. For instance, on Hera, the git restore function will not work due to:

+ [[ restore = \r\e\s\t\o\r\e ]]
+ git restore --staged regression
git: 'restore' is not a git command. See 'git --help'.

If we want this script to work on all machines, then additional work will need to be done.

@emilyhcliu
Copy link
Contributor

@RussTreadon-NOAA @MichaelLueken-NOAA
The scrtip is for removing/restoring GSI components while building GSI based on the provided rlist (list of components).
I see no issues with these changes. However, @MichaelLueken-NOAA pointed out that the git restore may not work for HERA.
I suggest that we just make it work for WCOSS-2 for this PR and leave the changes for HERA and other machine for future work.
What do you think?

@RussTreadon-NOAA
Copy link
Contributor Author

Hi @RussTreadon-NOAA. Do we want the functionality of ush/prune_4nco_global.sh on all machines, or just WCOSS2?

I ask because the version of git dictates the command required to restore the deleted contents. For instance, on Hera, the git restore function will not work due to:

+ [[ restore = \r\e\s\t\o\r\e ]]
+ git restore --staged regression
git: 'restore' is not a git command. See 'git --help'.

If we want this script to work on all machines, then additional work will need to be done.

Thank you, @MichaelLueken-NOAA , for checking the prune function on other machines. Since ush/prune_4nco_global.sh is only intended for operational GFS DA implementations, I think it is sufficient that the script works on WCOSS2.

NOAA-EMC/GSI release/gfsda.v16.3.0 no longer accurately reflects the directories and files in NOAA-EMC/GSI develop. Several directories and files currently found in release/gfsda.v16.3.0 have been moved into other repositories. Depending how g-w builds the GFS package in the future, the need for ush/prune_4nco_global.sh may be greatly diminished, perhaps even totally removed.

The above noted, a google search identified git 2.23.0 as the version at which the restore command was added. A few more google searches identified a way to compare version strings. This scripting along with additional logic was added to ush/prune_4nco_global.sh so that it works on WCOSS2, hera, and orion. The revised script was committed to the forked release/gfsda.v16.3.0 at 5145660. I did not test the revised ush/prune_4nco_global.sh on other machines (e.g., jet, gaea, discover, 'cheyenne).

@emilyhcliu
Copy link
Contributor

emilyhcliu commented Aug 30, 2022

@RussTreadon-NOAA Can I merge this to the authoritative release/gfsda.v16.3.0?
@MichaelLueken-NOAA Do you have any concerns and suggestions for PR?

@RussTreadon-NOAA
Copy link
Contributor Author

@RussTreadon-NOAA Can I merge this to the authoritative release/gfsda.v16.3.0? @MichaelLueken-NOAA Do you have any concerns and suggestions for PR?

@emilyhcliu , While I do not see myself making any more changes to the forked release/gfsda.v16.3.0 or this PR, let's wait and hear from @MichaelLueken-NOAA before merging.

Copy link
Contributor

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@RussTreadon-NOAA @emilyhcliu
Thank you very much, @RussTreadon-NOAA, for updating the script to work on machines other than WCOSS2. I am fine with moving forward with merging this PR to the authoritative release/gfsda.v16.3.0 branch. Once this work has been merged, I will re-cut the gfsda.v16.3.0 tag.

@RussTreadon-NOAA
Copy link
Contributor Author

Thank you, @MichaelLueken-NOAA, for your review and approval. @emilyhcliu , we can merge this PR into the authoritative release/gfsda.v16.3.0.

@emilyhcliu emilyhcliu merged commit 98184a0 into NOAA-EMC:release/gfsda.v16.3.0 Aug 30, 2022
@emilyhcliu
Copy link
Contributor

@MichaelLueken-NOAA I just merged this PR to release/gfsda.v16.3.0.
We are ready to re-cut the gfsda.v16.3.0 tag!

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

Successfully merging this pull request may close these issues.

3 participants