`ref` & `hist` times in bias adjustment methods must match

Ouranosinc / xclim

Library of derived climate variables, ie climate indicators, based on xarray.

https://xclim.readthedocs.io/en/stable/

Apache License 2.0

333 stars 59 forks source link

`ref` & `hist` times in bias adjustment methods must match #1995

Open coxipi opened 1 week ago

coxipi commented 1 week ago

Pull Request Checklist:

[ ] This PR addresses an already opened issue (for bug fixes / features)
- This PR fixes #xyz
[ ] Tests for the changes have been added (for bug fixes / features)
- [ ] (If applicable) Documentation has been added / updated (for bug fixes / features)
[ ] 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?

Add a method to bias adjustment classes to raise an error if ref and hist times do not match

Does this PR introduce a breaking change?

Other information:

coxipi commented 1 week ago

it keeps saying that some checks were not successful, but I don't see any red x, showing the failures.

aulemahal commented 1 week ago

Weird, the previous CI shows some runs "cancelled". I don't see why they were cancelled...

Zeitsperre commented 1 week ago

It's a configuration issue. I'll see what I can do.

coxipi commented 5 hours ago

So all this discussion about conflict values of time with map_blocks if times are not maching:

I guess that's why NpdfTransform had:

                "ref": ref.rename(time="time_hist"),
                "hist": hist.rename(time="time_hist"),
                "sim": sim,

I'll try and see if the class Adjust could handle this on its own to also avoid the problem in other classes. I recently had this problem when trying to use dOTC.

For MBCn, I didn't see this with the groupies-like implementation since it doesn't use map_groups. I'm not sure how it worked for my map_groups implementation though , I feel it should have failed for this specific reason above.