Skip to content

Add rainfall depth to rate converter#2122

Open
Simon Osborne (mo-sro) wants to merge 10 commits into
mainfrom
convert_rainfall_amount_to_rate
Open

Add rainfall depth to rate converter#2122
Simon Osborne (mo-sro) wants to merge 10 commits into
mainfrom
convert_rainfall_amount_to_rate

Conversation

@mo-sro
Copy link
Copy Markdown
Contributor

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.
  • Ensure rose-suite.conf.example has been updated if new diagnostic added.
  • 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

Coverage

Copy link
Copy Markdown
Member

@jfrost-mo James Frost (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.

Useful functionality for enabling rainfall comparison. It needs tests adding, and ideally should be made non-Cardington specific, so other data can make use of it.

Comment thread src/CSET/operators/misc.py Outdated
Comment thread src/CSET/operators/misc.py Outdated
Comment thread src/CSET/operators/misc.py Outdated
Comment thread src/CSET/operators/misc.py Outdated
Comment thread src/CSET/operators/misc.py Outdated
Comment thread src/CSET/operators/misc.py Outdated

# --- convert depth -> rate ---
# mm / s == kg m-2 s-1
cube.data = cube.core_data() / duration_seconds
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
cube.data = cube.core_data() / duration_seconds
cube = cube / duration_seconds

I believe this will perform the calculation while maintaining lazy data.

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.

Does your suggestion create a new cube? Is this risky? Perhaps it should be
cube.data = cube.data / duration_seconds
instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We will want a test for this function in tests/operators/test_misc.py. Maybe one for it successfully running, then one for each of the errors raised.

Copy link
Copy Markdown
Contributor Author

@mo-sro Simon Osborne (mo-sro) May 27, 2026

Choose a reason for hiding this comment

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

Tests added: seven in total. This number seems necessary given how the function has changed. I have generated a set of tiny .nc files of synthetic data rather than create data on the fly. Is this appropriate? Perhaps you need to "test the tests" ??

Simon Osborne (mo-sro) and others added 9 commits May 27, 2026 08:30
Check that time units are in seconds, convert if necessary.
Make function work for all datasets containing a rainfall amount(accumulation).
use iter_maybe

Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
check cube has time coords; then check if time.bounds exists; if not, work out dt's and hence duration_seconds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants