Improve partition #504
Improve partition #504
Conversation
# Conflicts: # xscen/spatial.py
…partition # Conflicts: # CHANGELOG.rst
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
tests/test_biasadjust.py
Outdated
| @@ -47,11 +47,10 @@ def test_basic_train(self, var, period): | |||
|
|
|||
| def test_preprocess(self): | |||
|
|
|||
| dref360 = self.dref.convert_calendar("360_day", align_on="year") | |||
|
|
|||
| dhist360 = self.dhist.convert_calendar("360_day", align_on="year") | |||
There was a problem hiding this comment.
This test failed because ref had s 720 and sim a 730 shape, but actually it should have always failed. The function assumes that the simulation will always always a smaller or equal calendar to the ref ( ref don't have 360 day cal).
In the previous version, we were "converting" the 360 day cal of the ref to noleap, but the function doesn't add values if you don't specify it ( which we don't because we assume that we are always only removing dates). Hence, it stays 720 long and fails.
In this version, sim is 360 and ref is convert to 360 as well. Both are 720 long
There was a problem hiding this comment.
but I am now noticing that there is another issue with adpat_freq
There was a problem hiding this comment.
The problem was in xclim : https://github.com/Ouranosinc/xclim/pull/2038/files
I removed the test temporarily.
Pull Request Checklist:
number) and pull request (:pull:number) has been added.What kind of change does this PR introduce?
build_partition_dataDoes this PR introduce a breaking change?
yes, I removed the indicator_kw argument. I realized it was too heavy to do everything in one shot.
Other information:
Developped for "On the importance of the reference data: Uncertainty partitioning of bias-adjusted climate simulations over Quebec" by Lavoie et al. (submitted in 2024) (presented at the last jamboree)
Code for the paper here: https://github.com/Ouranosinc/partition