-
Notifications
You must be signed in to change notification settings - Fork 168
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
Refactor gridded wave post #3014
base: develop
Are you sure you want to change the base?
Refactor gridded wave post #3014
Conversation
@WalterKolczynski-NOAA I think this PR is ready to review. However, I don't know much about editing the |
@AntonMFernando-NOAA do you have a test case with develop and then a test case with this branch? |
@MatthewMasarik-NOAA , Since you are working on this script (scripts/exgfs_wave_post_gridded_sbs.sh) related to the GFS ops support, would you please take a look at the changes and check if this change would possibly solve the ops failure issue as well? |
Here is the branch started: https://github.com/MatthewMasarik-NOAA/global-workflow/tree/fix/wave-post. The last commit on it may be critical in addressing the post failure. Refactoring this loop was the longer term goal. |
|
@sbanihash I can't say how the changes to that script made here would change the overall program execution. I think the length and complexity of the main loop make it difficult to reason about it by inspection. That is why I thought refactoring was a good direction. |
@AntonMFernando-NOAA - Okay I can't go into your directory b/c of permissions, but essentially you are going to want to make sure you still have the same number of files in the gridded wave post output and that the files are the same before and after your changes. Also, while this breaks up the job into multiple parts - which is awesome! - we'll likely have other parts of this code that have "waiting" that likely need to be removed. That will require additional time that I do not have - but hopefully @sbanihash and @MatthewMasarik-NOAA can review that sooner than I'll be able to get to it. |
@AntonMFernando-NOAA please sync your branch with |
I'm concerned about rocoto not being able to handle the additional tasks for long forecasts. Maybe the merge should be delayed until a rocoto workaround is merged. |
8b9d5d1
to
720eb4c
Compare
…nando-NOAA/global-workflow into feature/wave_post_grid
@JessicaMeixner-NOAA Did a |
I have looked at the diff file that @AntonMFernando-NOAA has sent and communicated some more information that I would need to review the PR with him. He is working on running it on Hera so that we could take a closer look at it today. Will post here when we know more. Thanks! |
Description
The gridded wave post (wavepostsbs) script currently loops over all forecast hours and acts as its own post manager. Other component post and product scripts operate on one forecast time. This PR will update the wave post to match other components, that creates a bunch of short jobs instead of one long job (per member).
Resolves #2290
Type of change
Change characteristics
How has this been tested?
Checklist