-
Notifications
You must be signed in to change notification settings - Fork 205
Fix 06z gfs_sfcanl jobs failing due to a bug in CCPP #4389
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
Fix 06z gfs_sfcanl jobs failing due to a bug in CCPP #4389
Conversation
ush/parsing_ufs_configure.sh
Outdated
| local MOM6_OUTPUT_DIR="${MOM6_OUTPUT_DIR:-./MOM6_OUTPUT}" | ||
| local MOM6_RESTART_DIR="${MOM6_RESTART_DIR:-./MOM6_RESTART}" |
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.
| local MOM6_OUTPUT_DIR="${MOM6_OUTPUT_DIR:-./MOM6_OUTPUT}" | |
| local MOM6_RESTART_DIR="${MOM6_RESTART_DIR:-./MOM6_RESTART}" | |
| local MOM6_OUTPUT_DIR="./MOM6_OUTPUT" | |
| local MOM6_RESTART_DIR="./MOM6_RESTART" |
This isn't an option. The workflow will always write to this space. The other instances where this variable is set in this manner is ush/parsing_namelists_FV3.sh
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.
I agree, I don't think this should be configurable. This is assumed other places.
I also see that ufs-community/ufs-weather-model@6f22f57...f8b0802 we have a MOM6_OUTPUT_FH defined, which is defined in forecast_predet.
I'm assuming those are consistent definitions?
aerorahul
left a comment
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.
just one suggestion, looks good.
JessicaMeixner-NOAA
left a comment
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.
We should run a S2S test of some sort to make sure ocean output looks okay given the changes. I didn't follow exactly when those changes went in. @jiandewang or @dpsarmie might know more about that the MOM6_OUTPUT_FH to make sure that is as expected here now.
ush/parsing_ufs_configure.sh
Outdated
| local MOM6_OUTPUT_DIR="${MOM6_OUTPUT_DIR:-./MOM6_OUTPUT}" | ||
| local MOM6_RESTART_DIR="${MOM6_RESTART_DIR:-./MOM6_RESTART}" |
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.
I agree, I don't think this should be configurable. This is assumed other places.
I also see that ufs-community/ufs-weather-model@6f22f57...f8b0802 we have a MOM6_OUTPUT_FH defined, which is defined in forecast_predet.
I'm assuming those are consistent definitions?
|
|
Yes. And looking a bit further, this variable should be "FHOUT_OCN" and not MOM6_OUTPUT_FH. |
ClaraDraper-NOAA
left a comment
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.
I haven't tested it, but can confirm that this PR brings the needed update to CCPP/physics into UFS_UTILS to fix the gfs_sfcanl issue. It also updates the CCPP/physics used by the model to the same hash.
Note that updating the ufs_model and ufs_utils brings in many additional changes in addition to the gfs_sfcnl issue.
446b134
|
All tests passed on Ursa. I will run a develop C96C48mx500 case to verify the MOM6 output, then launch CI on all platforms. |
|
I verified the 6-hour MOM6 forecast for the >cmp /scratch3/NCEPDEV/stmp/David.Huber/rt_4389/COMROOT/C96C48mx500_S2SW_cyc_gfs_4389/gfs.20211220/18/model/ocean/history/gfs.t18z.6hr_avg.f120.nc /scratch3/NCEPDEV/global/role.glopara/GFS_CI_CD/URSA/BUILDS/GITLAB/nightly_6ada5183_010726/RUNTESTS/COMROOT/C96C48mx500_S2SW_cyc_gfs_6ada5183-6840/gfs.20211220/18/model/ocean/history/gfs.t18z.6hr_avg.f120.nc
> echo $?
0Proceeding with CI testing. |
|
All tests passed on all platforms. Requesting final approvals to merge. |
| dep_dict = {'type': 'task', 'name': f'{self.run}_fbwind'} | ||
| deps.append(rocoto.add_dependency(dep_dict)) | ||
|
|
||
| if self.options['do_wave']: |
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.
This is okay to add but we don't actually archive any of the awips files for waves to my knowledge, so I don't think this is actually needed.
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.
Good to know! I'll open an issue to remove this tarball/job.
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.
Opened issues #4454 to retire AWIPS waves archiving.
…lobal-workflow-cloud into feature/use_container_prefix * 'feature/use_container_prefix' of github.com:NOAA-EPIC/global-workflow-cloud: sync with EMC build_all.sh fix Fix 06z gfs_sfcanl jobs failing due to a bug in CCPP (NOAA-EMC#4389) Build on compute or login nodes from one build script. (NOAA-EMC#4380) Refactor build_compute.py to optionally support node exclusion (NOAA-EMC#4447) Fix incorrect branching in bash code anl (NOAA-EMC#4442) Update of GitHub Copilot Instructions file for current MCP/RAG system (NOAA-EMC#4397) Fix reviewdog arg name (NOAA-EMC#4441) Fix bash code analysis issues (NOAA-EMC#4440) Enable gcafs tests on Ursa (NOAA-EMC#4439) Initialize LETKF solver after running observer (NOAA-EMC#4424) Move C96mx025 case to sfsv1 (NOAA-EMC#4435) Update HPC support levels. (NOAA-EMC#4430) Adding GCAFS products v2 (NOAA-EMC#4407) Feature/task template (NOAA-EMC#4427) Run GEMPAK jobs on prepost nodes (NOAA-EMC#4411) Build the GDASApp with 8 cores on Gaea again
Description
This fixes a bug in CCPP that prevented the 06Z
gfs_sfcanljob from running global_cycle on the 03Z IAU time. The code change now allows the input hour to be any integer between 0 and 23.Resolves #4364
Refs #4408 (partially resolves but a full investigation is needed)
Type of change
Change characteristics
How has this been tested?
Checklist