Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Input requirements break xs.ensemble_stats #298

Open
1 task done
RondeauG opened this issue Dec 6, 2023 · 0 comments
Open
1 task done

Input requirements break xs.ensemble_stats #298

RondeauG opened this issue Dec 6, 2023 · 0 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@RondeauG
Copy link
Collaborator

RondeauG commented Dec 6, 2023

Setup Information

  • xscen version: 0.7.21
  • xclim version: 0.47

Description

Currently, xs.ensemble_stats tries to handle multiple functions in xclim.ensembles that do not necessarily have compatible input requirements. Some examples:

  • mean_std_max_min and ensemble_percentiles compute their statistics along realization. Thus, time should already have been aggregated (e.g. through a 30-year mean). --> Works well with horizons.
  • robustness_factors with ref=None also only works across realization, and considers the input to be pre-computed deltas where time has been aggregated. --> Works well with horizons.
  • robustness_factors with ref=xr.DataArray needs to have a time axis and will average time itself. I think this is because some tests need daily or yearly data. --> Completely breaks the concept of horizons.
  • Other functions in xclim.ensembles can also be called in theory, but diverge so much from what we currently have implemented that they probably don't work at all.

Steps To Reproduce

No response

Additional context

xs.ensemble_stats performs no checks and never warns the user if the list of tests is incompatible with the data provided.

At minimum, we should add that. Better yet, we should try to find a way to support that, or split xs.ensemble_stats in multiple functions.

Contribution

  • I would be willing/able to open a Pull Request to address this bug.
@RondeauG RondeauG added bug Something isn't working enhancement New feature or request labels Dec 6, 2023
This was referenced Dec 6, 2023
Open
Merged
RondeauG added a commit that referenced this issue Dec 7, 2023
@RondeauG
Support robustness functions in xs.ensemble_stats (#299)
15f054a 
<!-- Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - No.
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features).
- [x] (If applicable) Tests have been added.
- [x] This PR does not seem to break the templates.
- [x] CHANGES.rst has been updated (with summary of main changes).
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added.

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

* You can now directly provide an ensemble dataset to
`xs.ensemble_stats`.
* `xs.ensemble_stats` now supports the robustness-related functions from
`xclim`.
* Small bugfix in `publish_release_notes`

### Does this PR introduce a breaking change?

* Removed explicit support for `ref` as it was implemented, since it
actually was completely broken... See #298.

### Other information:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@RondeauG