Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update dependency check for ice netcdf file for GEFS #2721

Closed
aerorahul opened this issue Jun 26, 2024 · 7 comments · Fixed by #2809
Closed

Update dependency check for ice netcdf file for GEFS #2721

aerorahul opened this issue Jun 26, 2024 · 7 comments · Fixed by #2809
Assignees
@EricSinsky-NOAA
Labels
bug Something isn't working

Comments

@aerorahul
Copy link
Contributor

What is wrong?

#2561 introduced a dependency check for ice netcdf file for GEFS via the use of this script. Rocoto provides an option for checking if the file exists together with the age of the file and should be utilized instead of this script.

What should have happened?

Use Rocoto <datadep age=120> to check if the file exists and is older than 2 mins, instead of calling a script via <sh> command.

What machines are impacted?

All or N/A

Steps to reproduce

Create the xml and look at the gefs ice product dependency section.

Additional information

None

Do you have a proposed solution?

Use Rocoto's datadep tag to check for dependencies.
@aerorahul aerorahul added bug Something isn't working triage Issues that are triage labels Jun 26, 2024
@EricSinsky-NOAA
Copy link
Contributor

EricSinsky-NOAA commented Jun 26, 2024 •
edited
Loading

Issue #2674 creates some complications in the ice_prod dependencies which led me to create this script. The reason why I got rid of the <datadep age=120> dependency is because in cases where FHOUT_ICE_GFS = 24 and cycle=t12z, the lead hours in the fhr variable (in the xml file) in the ice_prod task do not match the lead hours in the ice hist filenames. This means the following file dependency would never be satisfied in these particular cases: [email protected]_avg.f#fhr#.nc. That is why I removed that dependency.

For example, let's say that FHOUT_ICE_GFS=24 and cyc=12. For the ice_prod task, we have an fhr variable: <var name="fhr">024 048 072 096 120</var>
If we were to keep this filename dependency
(<datadep age="120"><cyclestr>&ROTDIR;/gefs.@Y@m@d/@H/mem#member#/model_data/ocean/history/[email protected]_avg.f#fhr#.nc</cyclestr></datadep>), the dependency would never be satisfied because these lead hours in the fhr variable are not written in the ice hist filenames. The ice hist filenames would have lead hours 036, 060, etc.

@EricSinsky-NOAA
Copy link
Contributor

Here is the discussion we had about this in the PR: #2561 (comment)

@EricSinsky-NOAA EricSinsky-NOAA mentioned this issue Jun 26, 2024
Open
@EricSinsky-NOAA
Copy link
Contributor

EricSinsky-NOAA commented Jun 26, 2024 •
edited
Loading

@aerorahul I am creating a branch to restore the rocoto <datadep age=120> for the ice_prod task and remove check_ice_netcdf.sh. I think we have two options on how to proceed with this issue.

The first option is to submit a PR that only restores the rocoto <datadep age=120> and removes check_ice_netcdf.sh. Doing this would reintroduce issue #2674 (this would not affect the CI testing or the reforecast), but then I can submit a subsequent PR with a permanent fix for issue #2674 later on.

The second option is to submit one PR that fixes this issue and issue #2674.

Do you have a preference?

@EricSinsky-NOAA
Copy link
Contributor

I should clarify that restoring rocoto <datadep age=120> and removing check_ice_netcdf.sh does not so much reintroduce issue #2674, but instead the ice_prod task would be impacted by issue #2674.

@aerorahul
Copy link
Contributor Author

I think we can wait for the second option to be available.
Thanks!

@NeilBarton-NOAA
Copy link
Contributor

Note, ush/check_netcdf.sh needs to be removed from the dependencies of the ocean_prod tasks also. @aerorahul Do you want a new issue or combine with this one?

@WalterKolczynski-NOAA WalterKolczynski-NOAA removed the triage Issues that are triage label Jul 22, 2024
@EricSinsky-NOAA
Copy link
Contributor

I have been working on a branch that is intended to resolve this issue.

@EricSinsky-NOAA EricSinsky-NOAA mentioned this issue Aug 6, 2024
Merged
7 tasks
WalterKolczynski-NOAA pushed a commit that referenced this issue Sep 23, 2024
@EricSinsky-NOAA
Remove need for a ice_prod dependency check script (#2809)
fe57bb4 
# Description
The main purpose of this PR is to remove the need for an ice_prod
dependency check script `ush/check_ice_netcdf.sh`. The original purpose
of the ice_prod dependency check script is to check for special case
dependencies where `( cyc + FHMIN ) % FHOUT_ICE )) =! 0` (more details
on this issue can be found in issue #2674 ). A bugfix for these special
cases is expected to come from a PR in the ufs-weather-model.

Resolves #2721 
Refs #2721, #2674
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@EricSinsky-NOAA EricSinsky-NOAA

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove need for a ice_prod dependency check script EricSinsky-NOAA/global-workflow
4 participants
@aerorahul @WalterKolczynski-NOAA @EricSinsky-NOAA @NeilBarton-NOAA