Skip to content

Commit

Permalink
Fix indicator outputting delta degC (#1973)
Browse files Browse the repository at this point in the history
<!--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:
- [ ] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #xyz
- [x] Tests for the changes have been added (for bug fixes / features)
- [ ] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGELOG.rst has been updated (with summary of main changes)
- [ ] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

### What kind of change does this PR introduce?

Before this fix: an indicator that outputs "degC" but as a "temperature:
difference" would bug. The compute function would return a dataset with
proper units and proper "units_metadata" attribute and then the
indicator would try to convert the data to the expected units, but doing
so it would only give the units to `convert_units_to`. It would not give
the `units_metadata` attribute.

thus, `convert_units_to` was taking the compute function output and
getting "delta_degC", as expected, but then it was not able to convert
to "degC", of course.

This change passes the whole attrs to `convert_units_to` so it can
understand that the expected units are a difference.

### Does this PR introduce a breaking change?
No.

We have no current indicator that output "degC" as "difference" (dtr
outputs Kelvins), but the problem would emerge in custom indicators or
with `sdba.measures.bias` where units are not explicitly set, so the
"expected ones" are inferred from the compute function's output. Before
this fix, the `units_metadata` attribute was dropped at that point,
triggering the bug described above.


### Other information:
  • Loading branch information
aulemahal authored Oct 24, 2024
2 parents 5f990d4 + 506c699 commit c4e7f8c
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 3 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
Changelog
=========

v0.54 (unpublished)
v0.54.0 (unreleased)
--------------------
Contributors to this version: Éric (:user:`coxipi`).
Contributors to this version: Éric Dupuis (:user:`coxipi`), Pascal Bourgault (:user:`aulemahal`).

Bug fixes
^^^^^^^^^
* Conversion of units of multivariate DataArray is now properly handled in `sdba.TrainAdjust` and `sdba.Adjust`. There was a bug where the units could be changed before a conversion of the magntitudes could occur. (:pull:`1972`).
* Fix for indicators that output "delta" Celsius degrees. (:pull:`1973`).

v0.53.1 (2024-10-21)
--------------------
Expand Down
14 changes: 14 additions & 0 deletions tests/test_indicators.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,20 @@ def test_temp_unit_conversion(tas_series):
np.testing.assert_array_almost_equal(txk, txc + 273.15)


def test_temp_diff_unit_conversion(tasmax_series, tasmin_series):
tx = tasmax_series(np.arange(365) + 1, start="2001-01-01")
tn = tasmin_series(np.arange(365), start="2001-01-01")
txC = convert_units_to(tx, "degC")
tnC = convert_units_to(tn, "degC")

ind = xclim.atmos.daily_temperature_range.from_dict(
{"units": "degC"}, "dtr_degC", "test"
)
out = ind(tasmax=txC, tasmin=tnC)
assert out.attrs["units"] == "degC"
assert out.attrs["units_metadata"] == "temperature: difference"


def test_multiindicator(tas_series):
tas = tas_series(np.arange(366), start="2000-01-01")
tmin, tmax = multiTemp(tas, freq="YS")
Expand Down
2 changes: 1 addition & 1 deletion xclim/core/indicator.py
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ def __call__(self, *args, **kwds):

# Convert to output units
outs = [
convert_units_to(out, attrs["units"], self.context)
convert_units_to(out, attrs, self.context)
for out, attrs in zip(outs, out_attrs, strict=False)
]

Expand Down

0 comments on commit c4e7f8c

Please sign in to comment.