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

Spatial difference plots #1061

Merged
merged 9 commits into from
Jan 28, 2025
Merged

Spatial difference plots #1061

merged 9 commits into from
Jan 28, 2025

Conversation

jfrost-mo
Copy link
Member

@jfrost-mo jfrost-mo commented Jan 21, 2025

At long last this is ready to go into CSET. This PR adds a new misc.difference operator, a recipe and an include file exercising it.

The linking into the rose-edit was actually done in #1055, so that toggle will actually do something now.

Fixes #455

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.

@jfrost-mo jfrost-mo added the enhancement New feature or request label Jan 21, 2025
@jfrost-mo jfrost-mo self-assigned this Jan 21, 2025
Copy link
Contributor

github-actions bot commented Jan 21, 2025

Coverage

Base automatically changed from improve_rose_edit to main January 23, 2025 14:40
This uses the `cset_comparison_base` cube attribute to determine which
cube to use as the comparison base.

For now it can only take difference between the same validity times,
and it filters down to only the common times.
The bwr colour map is chosen to highlight differences.
This required adding some LFRic test data. This is a small cutout
over Exeter from an LFRic_apps vn1.1 run valid for 20220101T0000Z.
@jfrost-mo jfrost-mo requested review from daflack and jwarner8 and removed request for daflack January 24, 2025 12:36
@jfrost-mo jfrost-mo marked this pull request as ready for review January 24, 2025 12:36
Copy link
Contributor

@daflack daflack left a comment

Choose a reason for hiding this comment

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

Science review:

From a science perspective the documentation is clear. However, the recipe is less so - see comment. There do not appear to be any issues from a science implementation perspective. Code not ran as part of the science review and should be part of the technical review.

Approved from Science perspective.

Copy link
Contributor

@jwarner8 jwarner8 left a comment

Choose a reason for hiding this comment

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

Some minor suggestions, I will run off this branch later to do a few basic tests (noting your comment that colorbar diffs are limited to few variables and that you'll merge #1064 in after)

src/CSET/operators/_utils.py Outdated Show resolved Hide resolved
src/CSET/operators/misc.py Show resolved Hide resolved
src/CSET/operators/misc.py Show resolved Hide resolved
We instead just use the order of the latitude coordinate, which is
what we care about anyway.
We no longer need it, and ideally want code to be model agnostic.
@jfrost-mo jfrost-mo requested a review from jwarner8 January 27, 2025 16:34
Copy link
Contributor

@jwarner8 jwarner8 left a comment

Choose a reason for hiding this comment

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

Looks good, will run tomorrow and do sanity check now MASS #1084 fixes #1083

@jfrost-mo jfrost-mo merged commit 4e3a38d into main Jan 28, 2025
8 checks passed
@jfrost-mo jfrost-mo deleted the difference_plots branch January 28, 2025 09:09
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.

2D Difference / bias map plots
3 participants