Skip to content

[develop] Add correct logic for 3-digit forecast hours in UPP#607

Merged
MichaelLueken merged 1 commit into
ufs-community:developfrom
mkavulich:feature/issue_601_long_fcst_fix
Feb 10, 2023
Merged

[develop] Add correct logic for 3-digit forecast hours in UPP#607
MichaelLueken merged 1 commit into
ufs-community:developfrom
mkavulich:feature/issue_601_long_fcst_fix

Conversation

@mkavulich
Copy link
Copy Markdown
Collaborator

DESCRIPTION OF CHANGES:

Adding correct logic for 3-digit forecast hours in UPP. The filenames coming out of UPP are unfortunately hard-coded in that all forecast hours below 100 will have a 2-digit forecast hour in the filename, and beyond that will switch to a 3-digit forecast hour. The current workflow does not attempt to resolve this difference, resulting in UPP tasks failing after (despite the executable completing successfully). This change adds the correct logic to allow UPP tasks to succeed beyond 99 hours.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

TESTS CONDUCTED:

Ran a 108-hour forecast (using the config.yaml shown in #601) on Hera and Jet, confirmed that the fix works for 3-digit forecast hours. Ran the comprehensive test suite on Jet and no new failures were observed; all post tasks succeeded.

  • hera.intel
  • jet.intel
  • comprehensive tests (Jet)

DEPENDENCIES:

None

DOCUMENTATION:

None

ISSUE:

Fixes issue mentioned in #601

CHECKLIST

  • My code follows the style guidelines in the Contributor's Guide
  • I have performed a self-review of my own code using the Code Reviewer's Guide
  • My changes do not require updates to the documentation (explain).
    • It's a one-line change
  • My changes generate no new warnings
  • New and existing tests pass with my changes

Copy link
Copy Markdown
Collaborator

@christinaholtNOAA christinaholtNOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

0. In this case, it is not clear how to set the variable post_fhr used
in constructing the grib2 file names generated by UPP:
fhr = \"$fhr\""
post_fhr="${fhr}"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment here on WHY this is necessary might help the next person from going down the rabbit hole you described in personal communications.

Copy link
Copy Markdown
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkavulich This change looks good to me!

@MichaelLueken MichaelLueken linked an issue Feb 10, 2023 that may be closed by this pull request
@MichaelLueken MichaelLueken added run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests ci-hera-intel-WE Kicks off automated workflow test on hera with intel labels Feb 10, 2023
@venitahagerty venitahagerty removed the ci-hera-intel-WE Kicks off automated workflow test on hera with intel label Feb 10, 2023
@venitahagerty
Copy link
Copy Markdown
Collaborator

venitahagerty commented Feb 10, 2023

Machine: hera
Compiler: intel
Job: WE
Repo location: /scratch1/BMC/zrtrr/rrfs_ci/autoci/pr/1237037101/20230210173511/ufs-srweather-app
Build was Successful
Rocoto jobs started
Long term tracking will be done on 10 experiments
If test failed, please make changes and add the following label back:
ci-hera-intel-WE
Experiment Succeeded on hera: pregen_grid_orog_sfc_climo

@MichaelLueken
Copy link
Copy Markdown
Collaborator

The Jenkins tests successfully completed for non-Hera machines and the GHA Hera WE2E test successfully completed. Moving forward with merging.

@MichaelLueken MichaelLueken merged commit 4b77153 into ufs-community:develop Feb 10, 2023
@mkavulich mkavulich deleted the feature/issue_601_long_fcst_fix branch April 21, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RUN_POST tasks fail for forecasts longer than 99 hours

4 participants