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

Convert [length] into [length]/[time] when cell_methods and/or standard_name allows it #1205

Closed
1 task done
bzah opened this issue Oct 17, 2022 · 1 comment · Fixed by #1206
Closed
1 task done
Labels
enhancement New feature or request

Comments

@bzah
Copy link
Contributor

bzah commented Oct 17, 2022

Generic Issue

  • xclim version: 0.38

Description

There are precipitation datasets, such as E-OBS, that bear a [length] units (usually "mm") that could be interpreted as a [length]/[time] (usually "mm/day") when computing climate indices.

I think xclim.units.convert_units_to could handle such cases when the input is a DataArray.

Two parameters would help us find if this conversion would be safe:

  • if cell_methods contains "time: sum", it means it's an accumulation over time.
  • if the variable standard_name is the CF thickness_of_rainfall_amount:

"Amount" means mass per unit area. The construction thickness_of_[X_]rainfall_amount means the accumulated "depth" of rainfall i.e. the thickness of a layer of liquid water having the same mass per unit area as the rainfall amount.

Note that "cell_methods" seems to be missing from e-obs datasets, so it might not be the best idea to use that.


What do you think about that ?
If you are okay with adding this feature, I can implement it.

ref:

Code of Conduct

  • I agree to follow this project's Code of Conduct
@aulemahal
Copy link
Collaborator

aulemahal commented Oct 17, 2022

The conversion should be done with xc.core.units.amount2rate I think.

But indeed, an automatic conversion for some specific variables could be interesting. We already do automatic conversion between precipitation rate in [length]/[time] and [mass]/([area][time]).

EDIT: I am indeed doubtful that using cell_methods is a good idea. In my experience, this field is often missing or simply wrong.

@bzah bzah changed the title [enh] Convert "mm" into "mm/{freq}" when cell_methods and/or standard_name allows it Convert [length] into [length]/[time] when cell_methods and/or standard_name allows it Oct 18, 2022
@bzah bzah added the enhancement New feature or request label Oct 18, 2022
aulemahal added a commit that referenced this issue Dec 5, 2022
<!--Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #1205
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] HISTORY.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added
- [x] The relevant author information has been added to `AUTHORS.rst`
and `.zenodo.json`

### What kind of change does this PR introduce?
This PR adds standard-name-aware unit conversions. Those are listed in
`data/variables.yml` in a new "conversions" section. For now, the
`amount2rate` and the new `amount2lwethickness` are available (and
reverse operations).

`convert_units_to` will also perform those conversion automatically if
the change in dimensionality requires it and the input has a valid
standard name.

The new functions are often the equivalent of the "hydro" context. Both
are complementary: the hydro context is still to be used in indicators
for which the transformation is implicitly valid, no matter the CF
attributes.

This is necessary to be able to compute precipitation indices on
datasets that express RR as "thickness_of_rainfall_amount" instead of
the usual "precipitation_flux" such as the e-obs datasets.

I can't make these transformations available automatically in the
indices with the current structure state of `declare_units`, I'll look
into this in a future PR. For now, as shown in the edited `units.ipynb`
these "CF conversions" must be down explicitly beforehand.


### Does this PR introduce a breaking change?
Yes, the error `pint.errors.DimensionalityError` is no longer raised
when attempting to convert this kind of datasets described above into a
`[length] / [time]` compatible unit.
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 a pull request may close this issue.

2 participants