-
Notifications
You must be signed in to change notification settings - Fork 172
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 snow obs preprocessing job #2946
Update snow obs preprocessing job #2946
Conversation
Replace it with a direct read from BUFR at runtime.
workflow/rocoto/gfs_cycled_xml.py
Outdated
if self._app_config.do_jedisnowda: | ||
sdate_snocvr = self._base['SDATE'] | ||
edate_snocvr = self._base['EDATE'] | ||
interval_snocvr = to_timedelta(f"24:00:00H") |
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.
interval_snocvr = to_timedelta(f"24:00:00H") | |
interval_snocvr = to_timedelta('24H') |
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.
Made changes. Thanks.
task_dict = {'task_name': task_name, | ||
'resources': resources, | ||
'dependency': dependencies, | ||
'envars': self.envars, | ||
'cycledef': self.run.replace('enkf', ''), | ||
'command': f'{self.HOMEgfs}/jobs/rocoto/prepsnowobs.sh', | ||
'cycledef': cycledef, |
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.
'cycledef': cycledef, | |
'cycledef': 'gdas_prep_snocvr', |
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.
Made changes as suggested. Thanks.
|
||
deps = [] | ||
dep_dict = {'type': 'task', 'name': f'{self.run}prep'} | ||
deps.append(rocoto.add_dependency(dep_dict)) | ||
dependencies = rocoto.create_dependency(dep=deps) | ||
|
||
resources = self.get_resource('prepsnowobs') | ||
task_name = f'{self.run}prepsnowobs' | ||
cycledef = 'gdas_prep_snocvr' |
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.
cycledef = 'gdas_prep_snocvr' |
@jiaruidong2017 can you resolve conflicts and then, if all his comments are addressed, re-request review from @WalterKolczynski-NOAA ? |
…2017/global-workflow into feature/ims_snowcover
@WalterKolczynski-NOAA The I conducted the test run, and the run was successful. Would you please review this PR? Thanks. |
CI Failed on Hera in Build# 5
|
@jiaruidong2017 It looks like the dependencies are not quite right. The workflow stalled because the Task: enkfgdas_esnowrecen
...
dependencies
AND is not satisfied
gdas_prepsnowcover of cycle 202112201800 is not SUCCEEDED
...
Task can not be submitted because:
Dependencies are not satisfied I assume the global-workflow/workflow/rocoto/gfs_tasks.py Line 638 in 20b3b9c
to 'cycledef': 'gdas_prep_snocvr', Also, the dependencies for the global-workflow/workflow/rocoto/gfs_tasks.py Lines 596 to 601 in 20b3b9c
|
@DavidHuber-NOAA The |
OK, then the dependencies for just the |
@DavidHuber-NOAA I have updated the I didn't conduct the hybrid run in my test. Do you want me to run the hybrid test first or you submit a CI test directly? Please let me know. Thank you. |
@DavidHuber-NOAA I have just submitted a hybrid test run, and will let you know when it is done. |
@DavidHuber-NOAA The hybrid run succeeded as below. It is ready to have a CI test.
|
@jiaruidong2017 I see that there is a |
@DavidHuber-NOAA The Previously, the gfs run is at 00z, while the above gfs run in CI test is at 18z. Therefore, the current design of IMS data processing and DA at 00z will be never happened for the gfs runs. @CoryMartin-NOAA Do we need to make change to move IMS data processing and DA from 00Z to 18Z? |
@jiaruidong2017 no, for this CI test, if the GFS cycle is only at 18z, then there is no gfs_prepsnowcover. This is okay, since there will be a gdas_prepsnowcover that is tested. The 18z cycles for both GFS and GDAS will just assimilate in situ snow observations. |
Thanks @CoryMartin-NOAA for clarifying this. Once the gfs cycle is running at 00z cycle, the |
if there's no GFS forecast, there should be no GFS prepsnowcover job, but it seems like there is one showing up, right? That needs to be fixed |
@DavidHuber-NOAA I updated the
|
cycledef = 'gdas_prep_snocvr' | ||
if self.run in ['gfs']: | ||
cycledef = self.run | ||
|
||
resources = self.get_resource('prepsnowcover') | ||
task_name = f'{self.run}_prepsnowcover' | ||
task_dict = {'task_name': task_name, | ||
'resources': resources, | ||
'dependency': dependencies, | ||
'envars': self.envars, | ||
'cycledef': 'gdas_prep_snocvr', | ||
'cycledef': cycledef, |
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 this solution will only work if the gfs runs on just the 00z cycle. If the gfs forecast is to run on 06, 12, and/or 18z, then this job will be added to the mesh and will fail. I see two options.
- Add another
cycledef
:gfs_prep_snocvr
. Thecycledef
could then be defined here asf"{run}_prep_snocvr"
- Add an if block:
(pseudocode)
if run == 'gfs':
<add a taskvalid dependency for gdas_prepsnowcover>
<cycledef = 'gfs'>
else:
<cycledef = 'gdas_prep_snocvr'>
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 @DavidHuber-NOAA for your review. Per above test, the gfs runs at 18z cycle, the added gfs_prepsnowcover
job did nothing and will not stall the workflow. If the gfs forecast is to run on 06, 12, and/or 18z, the script scripts/exglobal_prep_snowcover.py
will control gfs_prepsnowcover
job doing nothing as below:
if SnowAnl.task_config.cyc == 0:
SnowAnl.prepare_IMS()
Therefore, the current settings won't cause any problems to the workflow.
I was thinking to add another cycledef
: gfs_prep_snocvr
, but I don't think it helps, because the gfs_prep_snocvr
should be the same as gdas_prep_snocvr
.
In addition, the option 2 you proposed above is the same with current setting as below:
cycledef = 'gdas_prep_snocvr'
if self.run in ['gfs']:
cycledef = self.run
@DavidHuber-NOAA Do you agree with me? Thank you very much.
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, I do agree with you now that I see the change you made in exglobal_prep_snowcover.py
. I think we would prefer to not have do-nothing jobs, but this would work for now.
@WalterKolczynski-NOAA what do you think?
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.
Agree that we want to eliminate do-nothing jobs.
For a solution, we need to answer some questions first::
- Is the goal to have the GFS job run once per day? Or should it really only run at 00z?
- Can it run every day at 00z even if there is no GFS otherwise in that cycle (assuming there are cycles with GFS)?
- What about if there is no GFS cycle on a given day? (For instance, GFS is running every 48 h)
- What is actually being done by this job in the GFS cycle that is different than GDAS? Does it need to be run again for GFS at all?
We had similar restraints with metp, and the answers were once a day and it could be run even if there was no GFS in that cycle, but only if there was GFS sometime that day. The solution there was to run metp at 18z every day if the GFS ran more frequently than once a day (even if GFS did not run at 18z), and on the gfs cycledef if the GFS was run less often than once/day.
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 @WalterKolczynski-NOAA for your reply. I think @CoryMartin-NOAA is best person to give more accurate answers.
@CoryMartin-NOAA Would you please answer these questions? Thanks.
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'd need to check to confirm, but I think, while it doesn't need to run for GFS and would be identical to GDAS, the GFS cycle runs 'first', so in ops this matters, but in R&D not so much. So perhaps we can use the GDAS 00z file in the GFS 00z analysis (but I don't know if this dependency is a good idea).
For your other questions:
- only at 00z, it's based on a L3 product
- yes, waste of resources but no harm otherwise
- either can skip it completely or run and produce a useless file
- See above for answer, it's identical, but dependencies could be tricky
@WalterKolczynski-NOAA with all of this discussion, do you think it makes sense to instead wrap this job into the |
@CoryMartin-NOAA The prepsnowcover job is designed to use a single processor and take less than 5 minutes. |
Discussed this with Cory. We've decided to go forward with his proposed plan to just add the snow obs processing to the snow analysis job, since it is fast and doesn't need many resources. That will allow us to avoid any cycledef/dependency issues. |
Yes, @jiaruidong2017 can you close this PR and create a new one where the snow cover processing is done at the beginning of the snow analysis job? |
@CoryMartin-NOAA Okay, I will proceed to work and create a new PR. |
Description
This PR modifies the workflow XML configuration so that the prep snow obs job only runs on the 00z cycle.
This PR also renames the
snow_obs
andsnowobs
to thesnowcover
for highlighting the input IMS data source as snow cover.Resolves (#2902)
Type of change
Change characteristics
How has this been tested?
Checklist