-
Notifications
You must be signed in to change notification settings - Fork 156
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 Jet support #537
Update Jet support #537
Conversation
Please refer to GSI How to make changes to the GSI code and GSI Code Management Policy for up to date procedures for processing NOAA-EMC/GSI PRs. Two peer reviews with approval are needed before a member of the Handling Review team can act on this PR. |
@RussTreadon-NOAA I do not have permission to assign reviewers. I am also not sure who a good choice would be. Could you make a recommendation? |
@DavidHuber-NOAA , do you know which developers run |
@jkhender and @nshahroudi would you be able to review this GSI PR to update support for Jet? |
@DavidHuber-NOAA Sorry, I do not run GSI so probably not the best person to do a review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. A few minor comments.
regression/regression_var.sh
Outdated
@@ -33,7 +33,7 @@ if [[ -d /glade ]]; then # Cheyenne | |||
export machine="Cheyenne" | |||
elif [[ -d /scratch1 ]]; then # Hera | |||
export machine="Hera" | |||
elif [[ -d /jetmon ]]; then # Jet | |||
elif [[ -d /mnt/lfs4 ]]; then # Jet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"/jetmon" would be more general. Although it does not happen often, but there are times when /lfs4 may not be mounted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the time that I was testing this, /jetmon was not available. I will add additional jet-specific checks in case /mnt/lfs4 is not mounted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to include /jetmon and /mnt/lfs1.
Thank you for the review @guoqing-noaa! |
a6a6f92
to
ea14f88
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Looks good Dave. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve given approval from two peer reviews.
Description
Updates the module file for Jet to 1) point to the EPIC-managed hpc-stack, and 2) update the format to match recent updates. The regression test CASES data has been refreshed on the system and regression_var.sh was updated to point to the new location. Fixes #529.
Type of change
How Has This Been Tested?
Regression tests were run on the system. All reproducibility tests passed. Only the following 4 failed for the mentioned reasons:
4d_envar_loproc_update
time thresholdnetcdf_fv3_regional_loproc_updat
scalabilityhwrf_nmm_d3_loproc_update
time thresholdrtma_hiproc_update
time threshold & scalabilityAdditionally, cycled global workflow tests were performed at C96/C48, C192/C96, and C384/C192.
Checklist