Sum is a sum (#1649)

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

* `to_agg_units` will not change the units when the aggregation is
`"sum"`.
* Fixed the units when the initial data has a temporal dimensionality
(ex "m/s") and an `"integral"` is performed. The output units were
previously not reduced, yielding things like "d m s-1".

### Does this PR introduce a breaking change?

Yes, but `select_resample_op` is rarely used by the user directly. And
we could consider that this is in fact a _fix_ from a previous ambiguous
situation. No internal calls to `to_agg_units` were using `"sum"`.

CHANGES.rst

@@ -41,6 +41,7 @@ Breaking changes
 * `black` formatting style has been updated to the 2024 stable conventions. `isort` has been added to the `dev` installation recipe. (:pull:`1626`).
 * The indice and indicator for ``winter_storm`` has been removed (deprecated since `xclim` v0.46.0 in favour of ``snd_storm_days``). (:pull:`1565`).
 * `xclim` has dropped support for `scipy` version below v1.9.0 and `numpy` versions below v1.20.0. (:pull:`1565`).
+* For generic function ``select_resample_op`` and ``core.units.to_agg_units``, operation "sum" will now return the same units as the input, and not implicitly be translated to an "integral". (:issue:`1645`, :pull:`1649`).
 
 Bug fixes
 ^^^^^^^^^

tests/test_atmos.py

@@ -280,7 +280,7 @@ def test_wind_power_potential_from_3h_series():
     power = out * 100
     power.attrs["units"] = "MW"
     annual_power = convert_units_to(
-        select_resample_op(power, op="sum", freq="D"), "MWh"
+        select_resample_op(power, op="integral", freq="D"), "MWh"
     )
     assert (annual_power == 100 * 24).all()
 

tests/test_units.py

@@ -22,6 +22,7 @@
     pint_multiply,
     rate2amount,
     str2pint,
+    to_agg_units,
     units,
     units2pint,
 )
@@ -317,3 +318,28 @@ def index(data: xr.DataArray, thresh: Quantified, dthreshdt: Quantified):
 
     with pytest.raises(ValidationError):
         index_full_mm("1 mm", "2 Pa", "3 mm/s")
+
+
+@pytest.mark.parametrize(
+    "in_u,opfunc,op,exp,exp_u",
+    [
+        ("m/h", "sum", "integral", 8760, "m"),
+        ("m/h", "sum", "sum", 365, "m/h"),
+        ("K", "mean", "mean", 1, "K"),
+        ("", "sum", "count", 365, "d"),
+        ("", "sum", "count", 365, "d"),
+        ("kg m-2", "var", "var", 0, "kg2 m-4"),
+        ("°C", "argmax", "doymax", 0, ""),
+    ],
+)
+def test_to_agg_units(in_u, opfunc, op, exp, exp_u):
+    da = xr.DataArray(
+        np.ones((365,)),
+        dims=("time",),
+        coords={"time": xr.cftime_range("1993-01-01", periods=365, freq="D")},
+        attrs={"units": in_u},
+    )
+
+    out = to_agg_units(getattr(da, opfunc)(), da, op)
+    np.testing.assert_allclose(out, exp)
+    assert out.attrs["units"] == exp_u

xclim/core/units.py

@@ -471,8 +471,8 @@ def to_agg_units(
         The original array before the aggregation operation,
         used to infer the sampling units and get the variable units.
     op : {'min', 'max', 'mean', 'std', 'var', 'doymin', 'doymax',  'count', 'integral', 'sum'}
-        The type of aggregation operation performed. The special "delta_*" ops are used
-        with temperature units needing conversion to their "delta" counterparts (e.g. degree days)
+        The type of aggregation operation performed. "integral" is mathematically equivalent to "sum",
+        but the units are multiplied by the timestep of the data (requires an inferrable frequency).
     dim : str
         The time dimension along which the aggregation was performed.
 
@@ -521,7 +521,7 @@ def to_agg_units(
     >>> degdays.units
     'K d'
     """
-    if op in ["amin", "min", "amax", "max", "mean", "std"]:
+    if op in ["amin", "min", "amax", "max", "mean", "std", "sum"]:
         out.attrs["units"] = orig.attrs["units"]
 
     elif op in ["var"]:
@@ -532,16 +532,22 @@ def to_agg_units(
             units="", is_dayofyear=np.int32(1), calendar=get_calendar(orig)
         )
 
-    elif op in ["count", "integral", "sum"]:
+    elif op in ["count", "integral"]:
         m, freq_u_raw = infer_sampling_units(orig[dim])
         orig_u = str2pint(orig.units)
         freq_u = str2pint(freq_u_raw)
         out = out * m
 
         if op == "count":
             out.attrs["units"] = freq_u_raw
-        elif op in ["integral", "sum"]:
-            out.attrs["units"] = pint2cfunits(orig_u * freq_u)
+        elif op == "integral":
+            if "[time]" in orig_u.dimensionality:
+                # We need to simplify units after multiplication
+                out_units = (orig_u * freq_u).to_reduced_units()
+                out = out * out_units.magnitude
+                out.attrs["units"] = pint2cfunits(out_units)
+            else:
+                out.attrs["units"] = pint2cfunits(orig_u * freq_u)
     else:
         raise ValueError(
             f"Unknown aggregation op {op}. "