Ouranosinc / xclim / 6436235218

90%

main: 92%

push

github

Optimize huglin and bedd for dask with flox (#1495)

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

Changes the use of `aggregate_between_dates` to a
`select_time(...).resample().sum()`. One of the "strength" of
`aggregate_between_dates` is the possibility to have those days change
from year to year. In `huglin_index` and
`biologically_effective_degree_days`, we are using fixed start and end
dates, so I was able to perform the same operation with a simpler
function.

This change makes xarray aware of our resampling-sum computation and
thus, performance optimizations implemented by `flox` are made
accessible!

In order to preserve the signature and functionality, I had to make
`select_time` a bit more flexible so that we can choose if the bounds
are inclusive or not. (`aggregate_between_dates` has a non-inclusive
end-bound, which `select_time` is inclusive by default).

The results are different, but with a relative error around 1e-7, my
guess is that this comes from the optimization itself (`dask`) and
should be considered noise.

I think calendar support is preserved. I have found issues with using
leap years, non-uniform calendars, etc. However, this change now allows
passing `02-30` as a start or end date if the input uses `360_day`.
Haha, pretty sure that's almost useless, but it's there anyway.

The biggest difference is that the `xclim.indices` versions do not mask
incomplete periods anymore. With `aggregate_between_dates`, if the
`end_date` did not exist in the period, the output would be NaN. This is
not the case with `select_time`. The indicator is unchanged since the
missing check will mask the incomplete period.

### Does this PR introduce a breaking change?
I don't think the last element in a breaking change because:
- Indicators should always be used (especially if missing data are an
issue)
- I prefer that the indice doesn't perform "checks" that the indicator
should be doing

### Other informat... (continued)

14 of 15 new or added lines in 3 files covered. (93.33%)

8390 of 9279 relevant lines covered (90.42%)

4.37 hits per line