Add chill portion and chill units

[saschahofmann]

Closes #1753

TODO:

• Tests for the changes have been added (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • Test make_hourly_temperature
  • Test chill_portions
  • Test chill_units
• CHANGELOG.rst has been updated (with summary of main changes)
  • Link to issue (:issue:number) and pull request (:pull:number) has been added
• Check make_hourly_temperature is working with other calendars than 360_day

What kind of change does this PR introduce?

• Adds a chill_portion indicator based on the Dynamic model
• Adds a chill_unit indicator based on the Utah model
• Adds a helper function to estimate hourly temperatures from daily tasmin and tasmax

[Zeitsperre]

This looks great, thanks so much! I'll leave another reviewer to give the OK, though.

[saschahofmann]

I am still trying to figure out what the functions should return. Right now, the chill_portions returns an aggregated value per year but chill_units a value per hour that can be aggregated at will. Ideally, they would work the same but chill_portions are significantly harder to aggregate since an hours value depends on the previous hour's value. This also has implications on computing chill portions for a season. For chill units, you can simply select the range of days or hours afterwards and aggregate but for chill portions you need to select the period before and I could imagine this messing with the ResamplingIndicator class that fills missing data (@aulemahal ?) so I guess I need to do it inside the function?

As you can see in the current implementation of chill_portions, I am doing a yearly grouping to get the values per year but this would fail if you're interested in a season crossing new years (e.g. Nov-Feb). We do a trick where we shift the day so the season is within a year but that's only easy if you assume a 360_day calendar.

Update: chill_units now resamples to a year as well and sums the values but the question above remains: what do you think would be the right mechanism to operate on only a selected time range?

[saschahofmann]

I am also not sure what the IndexingIndicator class exactly is doing?

injects "indexer" kwargs to subset the inputs before computation.

sounds a little like what I am suggesting to do but I couldn't find how exactly it works?

In general, I was expecting these classes to rename and add the units specified but it seems they working more as a check? e.g. I made cu the varname for the chill units index but after the operation it's still tas. I guess I need to manually rename it?

And finally about testing: I am currently adding tests to the TestAgroclimaticIndices class in test_indices.py but I can see that there are also tests in test_temperature.py. What's the use case for the two?

[saschahofmann]

For now, I am keeping the logic to only take certain day ranges outside the functions but I think it should be included. Happy to discuss a mechanism on how to do that if we decide to do so. Tomorrow, I will add the test for chill portions and I think its ready for review

[saschahofmann]

Ready for review @aulemahal

Open questions:

1. Do I need to also test the indicator functions or are the indices enough (test_temperature.py)?
2. What's the cell_method argument for an indicator?
3. Which indicator class should I use?
4. Should the indices somehow handle picking the yearly season for which to compute the indices?

[aulemahal]

1. Generally our idea is to test the indices with explicit/synthetic data, as you did. And to test the indicators with real-world data (as in the atmosds fixture, which comes from ERA5-Land). But, the indicator tests can be minimal if they don't really add much. However, if you add the indexing functionality as discussed, I would suggest testing it in an indicator test.
2. Answered in the comments.
3. Same.
4. Not sure I understand. Is this related to the indexing/select_time ?

(For a better comprehension, read the comments in the order I made them, which is not in code order. 😅 )

[saschahofmann]

Ok I had no idea about the select_time function and the indexing its awesome!

I think there are several ways for me to implement it for chill portions but the question is how should I deal with seasons that span over years.

[aulemahal]

By "over years", you mean periods like "YS-JUL" that contain the change in calendar year ? I'm not I see how this is problematic for chill_portions as you don't explicitly make use of the datetime information ?

[saschahofmann]

Haha. Its only problematic if you had no idea that something like YS-JUL existed ... Alright, that solves a lot. Will implement the indexer solution for chill_portions and test it.