-
Notifications
You must be signed in to change notification settings - Fork 39
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
[production/RRFS.v1] Reorganization of NWGES directory structure and removal of SLASH_ENSMEM_SUBDIR #365
[production/RRFS.v1] Reorganization of NWGES directory structure and removal of SLASH_ENSMEM_SUBDIR #365
Conversation
@MatthewPyle-NOAA Having a NWGES directory structure similar to COMOUT also makes sense to me if we want to go that route. So for example we could have directories within nwges like this instead of the current ${PDY}${cyc} directories: |
@BenjaminBlake-NOAA I like that overall structure |
@MatthewPyle-NOAA Sounds good to me as well. I've updated my PR to reflect that structure and tested the changes. |
FWIW, I do use a GESROOT definition in the HREF system. I know I was surprised that NCO doesn't really talk about it in the production standards - need to dig out some old e-mail conversations with Steven Earle about it. But in HREF I do stuff like: export GESROOT=${GESROOT:-$(compath.py $envir/$NET/${href_ver})/nwges} |
I think using GESROOT makes sense and I'll change NWGES to GESROOT. The compath.py stuff can be added later but that should be the eventual goal. |
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.
Generally looked good to me - but did have a comment about spinup cycles.
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.
One comment that applies to RUN_POST as well - I don't believe there are "spinup" cycles for the ensemble forecasts.
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.
You're right, thanks for catching that. I'll update those J-jobs.
@BenjaminBlake-NOAA Did you retest with your latest changes in this PR? |
@MatthewPyle-NOAA I did test the GESROOT changes and they worked well, but I didn't run any tests for the more recent spinup changes. Would you like me to run any more tests for those? |
No need to run more tests at this point - will approve and get it merged. |
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, but a complete test will require testing the full system. So will merge now so as not to hold things up.
DESCRIPTION OF CHANGES:
$GESROOT
is now used instead of ${NWGES} for the path to the nwges directory.TESTS CONDUCTED:
These changes were tested with the non-DA engineering test and the RRFS fire weather workflow.
Machines/Platforms:
Test cases:
ISSUE: