Skip to content

Verification fixes#205

Merged
maddenp-cu merged 6 commits into
mainfrom
vx-fix
Jul 1, 2026
Merged

Verification fixes#205
maddenp-cu merged 6 commits into
mainfrom
vx-fix

Conversation

@maddenp-cu

Copy link
Copy Markdown
Contributor

Description:

Fixes #185. In manual tests on Ursa, diff plots are no longer blank, there are no errors in the postwxvx.log files, and the expected number of plots are produced.

Type of change:

  • Bug fix

Area(s) affected

  • nested_eagle workflow
  • Verification / evaluation (via WXVX)
  • Config (YAML)
  • Plotting / post-processing

Commit Requirements:

  • This PR addresses a relevant NOAA-EPIC/EAGLE issue (if not, create an issue); a person responsible for submitting the update has been assigned to the issue (link issue)
  • Fill out all sections of this template.
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have made corresponding changes to the system documentation if necessary
  • I checked whether this PR requires subcomponent PRs and completed the subcomponent checklist below

Testing / Verification:

  • I ran and/or verified the changes (or provided a test plan) -- ran Quickstart pipeline steps manually on Ursa, and CI tests will pass before commit.

@maddenp-cu maddenp-cu self-assigned this Jul 1, 2026
dir=$RUNROOT/visualization/grid2grid/$extent/plots-spatial-stats
count=$(find $dir -type f -name "*.png" | wc -l)
expected=8
expected=32

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The updated configuration calls for verification to start at the first non-zero leadtime, so for six-hourly forecasts to a max leadtime of 24, we have 4 forecasts (6, 12, 18, 24) x 8 forecast variables = 32 diff plots expected.

Comment thread config/base.yaml
inference_start: !datetime 2022-02-03T12
inference_stop: !datetime '{{ app.time.inference_start }}'
leadtime: 24
max_leadtime: 24

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought this updated name might better reflect the fact that inference produces forecasts at multiple leadtimes, not just one, which, to me, the term leadtime implies. I hope that max_leadtime implies that there are multiple leadtimes.

Comment thread config/base.yaml
Comment on lines +193 to +194
start: !int '{{ val.step }}'
end: !int '{{ app.time.max_leadtime }}'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These should be tied to the leadtime range of the overall configuration.

Comment thread config/base.yaml
validtime: time
leadtimes:
start: '{{ app.time.leadtime }}'
start: '{{ app.time.step }}'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verification should start at the first non-zero leadtime.

Comment thread config/base.yaml
meta:
workdir: '{{ val.vx.rundir }}/TRUTH/EXTENT'
grids: '{{ val.vx.rundir }}/TRUTH/EXTENT/grids'
max_leadtime: !int '{{ app.time.max_leadtime }}'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Technical detail: This value needs to be copied here from the app.time block so that it is available when wxvx runs and no longer has access to the full configuration but only to one specific wxvx: block.

Comment thread config/global.yaml
forecast:
name: global
path: '{{ meta.post_files }}/global.{{ yyyymmdd[:4] }}-{{ yyyymmdd[4:6] }}-{{ yyyymmdd[6:8] }}T{{ hh }}.{{ "%02d" % fh }}h.nc'
path: '{{ meta.post_files }}/global.{{ yyyymmdd[:4] }}-{{ yyyymmdd[4:6] }}-{{ yyyymmdd[6:8] }}T{{ hh }}.{{ "%02d" % meta.max_leadtime }}h.nc'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here and in several similar diffs that follow, respect the fact that inference writes forecasts for multiple leadtimes to a single netCDF file that includes the max-leadtime value in its filename. This lets wxvx find all of the forecasts in the appropriate, single file.

@maddenp-cu
maddenp-cu marked this pull request as ready for review July 1, 2026 19:20
@maddenp-cu
maddenp-cu requested a review from a team as a code owner July 1, 2026 19:20
@github-actions github-actions Bot added ci-running CI is running on this pull request and removed ci-running CI is running on this pull request labels Jul 1, 2026
@maddenp-cu
maddenp-cu deployed to eagle-ursa July 1, 2026 19:57 — with GitHub Actions Active
@github-actions github-actions Bot added the ci-running CI is running on this pull request label Jul 1, 2026

@mariahpope mariahpope left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks great, and exactly what I had to do to get things to run... but you did it elegantly instead of hard coding like me :)

Lets see if CI passes, with more plots happening now I want to make sure that the slurm wall clock is enough time (it should be). I only ran into this when doing the full 240 hours and a lot more variables. But, something to double check and CI will tell us.

As long as everything passes I am good to merge this!

@github-actions github-actions Bot removed the ci-running CI is running on this pull request label Jul 1, 2026
@maddenp-cu

Copy link
Copy Markdown
Contributor Author

Lets see if CI passes, with more plots happening now I want to make sure that the slurm wall clock is enough time (it should be). I only ran into this when doing the full 240 hours and a lot more variables. But, something to double check and CI will tell us.

@mariahpope CI passed, so I think we're good for now. Someday it would be nice to try to scale wallclock values based on the quantity of work to be done.

@maddenp-cu
maddenp-cu merged commit e30e612 into main Jul 1, 2026
5 checks passed
@maddenp-cu
maddenp-cu deleted the vx-fix branch July 1, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Visualization step producing blank basic plots

4 participants