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.

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

Various QuickLook fixes and enhancements of surface plot functionality. #1122

Merged
merged 35 commits into from
Feb 13, 2025

Conversation

jwarner8
Copy link
Contributor

@jwarner8 jwarner8 commented Feb 7, 2025

Addresses #1062.

  • Fixes missing stash conversions from Pauls list for a full set of diagnostics for RAL3-LFRic comparisons
  • Adds regridding capability for a limited set of variables that use different grid staggerings in the UM (wind)
  • Adds various callbacks to promote long_name, fix radiation and lightning time points.
  • Adds support for rotated GeogCS grids (iris copes with this OK and produces sensible results)
  • Add cell method constraint for surface_variables.
  • Updates tests

Contribution checklist

Aim to have all relevant checks ticked off before merging. See the developer's guide for more detail.

  • Documentation has been updated to reflect change.
  • New code has tests, and affected old tests have been updated.
  • All tests and CI checks pass.
  • Ensured the pull request title is descriptive.
  • Conda lock files have been updated if dependencies have changed.
  • Attributed any Generative AI, such as GitHub Copilot, used in this PR.
  • Marked the PR as ready to review.

@jwarner8 jwarner8 self-assigned this Feb 7, 2025
@jwarner8
Copy link
Contributor Author

jwarner8 commented Feb 7, 2025

Note: we currently look for variables with no cell_methods. This could become difficult if we want to compare time-meaned variables in the future.

Copy link
Contributor

github-actions bot commented Feb 7, 2025

Coverage

@jwarner8 jwarner8 changed the title Testing of all surface variables with CSET Testing of all variables with CSET Feb 7, 2025
@jwarner8
Copy link
Contributor Author

jwarner8 commented Feb 7, 2025

Nice (SURFACE VARIABLE TESTING) Capture

There were 6 failures, which were related to the following variables;
Capture

eastward_wind_at_10m. There is an extra gridpoint in the latitude direction (649 vs 648) in LFRic. Suspect due to whether variable on corners of cell, or center. Encountered before. Might need a callback fix (longer term needs a regrid).
nortward_wind_at_10m. Same as above.

toa_upward_longwave_flux_radiative_timestep. UM outputs at 1 minute past every hour, rather than on hour, so they are not directly compatible. Same with surface_net_longwave_flux_radiative_timestep, surface_downward_longwave_flux, surface_downward_shortwave_flux. So difference cant find any corresponding points to compare, and so produces no difference.

@jwarner8 jwarner8 changed the title Testing of all variables with CSET Testing all QuickLook surface level functionality Feb 10, 2025
@jwarner8 jwarner8 marked this pull request as ready for review February 10, 2025 10:49
src/CSET/operators/read.py Outdated Show resolved Hide resolved
@jwarner8
Copy link
Contributor Author

Update 11/02: All surface variables working, but need to update and tidy code and add new tests.

@Sylviabohnenstengel
Copy link
Contributor

surface_temperature has colourbar in degreeC and data in K. Need to change "surface_temperature": {
"max": 30.0,
"min": 20.0
},

into kelvin values

@jwarner8 jwarner8 changed the title Testing all QuickLook surface level functionality Various QuickLook fixes and enhancements of surface plot functionality. Feb 12, 2025
@jwarner8 jwarner8 added the enhancement New feature or request label Feb 12, 2025
@jwarner8 jwarner8 requested a review from jfrost-mo February 12, 2025 10:59
Copy link
Member

@jfrost-mo jfrost-mo left a comment

Choose a reason for hiding this comment

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

Overall looks good. There needs to be an additional test for regridding during the difference plot, and there are a few minor code suggestions.

src/CSET/operators/_stash_to_lfric.py Outdated Show resolved Hide resolved
src/CSET/recipes/generic_surface_histogram_series.yaml Outdated Show resolved Hide resolved
src/CSET/operators/misc.py Show resolved Hide resolved
tests/operators/test_misc.py Show resolved Hide resolved
src/CSET/operators/read.py Outdated Show resolved Hide resolved
src/CSET/operators/read.py Outdated Show resolved Hide resolved
src/CSET/operators/read.py Outdated Show resolved Hide resolved
src/CSET/operators/read.py Outdated Show resolved Hide resolved
@jwarner8 jwarner8 requested a review from jfrost-mo February 13, 2025 14:38
@jwarner8
Copy link
Contributor Author

Have added a test for regridding, not sure what to assert other than it didn't fail and successfully regridded, and thus returned a cube, so using type?

Copy link
Member

@jfrost-mo jfrost-mo left a comment

Choose a reason for hiding this comment

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

Suggestion for test conditions.

tests/operators/test_misc.py Outdated Show resolved Hide resolved
tests/operators/test_misc.py Outdated Show resolved Hide resolved
@jwarner8 jwarner8 requested a review from jfrost-mo February 13, 2025 15:19
Copy link
Member

@jfrost-mo jfrost-mo left a comment

Choose a reason for hiding this comment

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

Looking good!

@jfrost-mo jfrost-mo merged commit ac30e4c into main Feb 13, 2025
8 checks passed
@jfrost-mo jfrost-mo deleted the stash_fix branch February 13, 2025 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants