-
Notifications
You must be signed in to change notification settings - Fork 179
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
Allow increments to be added for ocean and ice cycling. #1308
Changes from all commits
311657d
364df4e
015e0bd
19c4748
46b1e7e
b04954b
55df592
acbeb76
6be7442
b8963ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
#! /usr/bin/env bash | ||
|
||
############################################################### | ||
if [[ "${DEBUG_WORKFLOW:-NO}" == "NO" ]]; then | ||
if [[ "${DEBUG_WORKFLOW}" == "NO" ]]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was a mistake to merge this change in... did anyone test? @jiaruidong2017 seems to be having an issue here |
||
echo "Loading modules quietly..." | ||
set +x | ||
fi | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,6 +120,14 @@ def link_files_from_src_to_dst(src_dir, dst_dir): | |
makedirs_if_missing(dst_dir) | ||
link_files_from_src_to_dst(src_dir, dst_dir) | ||
|
||
# First 1/2 cycle needs a MOM6 increment | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? is it not a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it's a |
||
incdir = f'{inputs.cdump}.{idatestr[:8]}/{idatestr[8:]}' | ||
incfile = f'{inputs.cdump}.t{idatestr[8:]}z.ocninc.nc' | ||
src_file = os.path.join(inputs.icsdir, incdir, 'ocean', incfile) | ||
dst_file = os.path.join(comrot, incdir, 'ocean', incfile) | ||
makedirs_if_missing(os.path.join(comrot, incdir, 'ocean')) | ||
os.symlink(src_file, dst_file) | ||
|
||
# Link ice files | ||
if do_ice: | ||
detdir = f'{inputs.cdump}.{rdatestr[:8]}/{rdatestr[8:]}' | ||
|
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.
So usually the MOM_input_template_* is a copy of what is in ufs-weather-model. Do these updates need a corresponding test in ufs-weather-model?
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.
Thanks for jumping in @JessicaMeixner-NOAA , I should have thought about adding you when I started modifying the mom namelist! Sorry.
Is there a MOM6 IAU test in the ufs-weather model?
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 think there is one here: https://github.com/ufs-community/ufs-weather-model/blob/develop/tests/tests/datm_cdeps_iau_gefs but it looks like the settings are different
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.
It seems close enough to me @JessicaMeixner-NOAA but up to you if you think it needs an update to the RT.
@aerorahul was trying to bring the naming nomenclature closer to what the atmosphere DA is doing, which does make sense.
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.
That sounds like a great idea to me too. I think it's important that we are testing in ufs-weather-model what we're actually going to be running and having consistency makes updating the model in the future easier. I believe there was talk at one point to actually point to the MOM_template files in ufs-eather-model instead of having a second copy of these, but I'm not sure if that's still going to be a future development for g-w or not. Either way, I just wanted to flag this as something to be aware of, but I don't think it should hold up this PR.
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.
Thanks @JessicaMeixner-NOAA
We can let this PR pass and I can open a follow up PR removing these files from global-workflow and using the ones from ufs-weather-model.
Would you be able to provide a branch of the ufs-weather-model that allows for this flexibility?
The ufs-weather-model RT parm/ files may need an update.
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'd need the following:
I can do all of the above, but it will be a while before I personally have time to do this. Again, there's no reason this PR should not proceed.
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.
Yes. 3. is on our radar.
We need atleast 1. for 3.
There is a lot more work in the ufs-weather-model than in the global-workflow to achieve this.
I can create issues in the global-workflow and ufs-weather-model and assign us respectively if that is ok?
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.
That sounds great. Just know this isn't anywhere near the top of my priority list at the moment.
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.
@JessicaMeixner-NOAA
#1312
ufs-community/ufs-weather-model#1613
have been created