Add Lafferty and Sriver partition #1529
Conversation
|
Right now, |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
@juliettelavoie Be sure to follow the steps in the README for testing things on the PR before merging to master: https://github.com/Ouranosinc/xclim-testdata/blob/main/README.md |
|
I'm feeling so-so regarding the fraction keyword. The issue I see is that there are a bunch of different figures typically created from those results, some using the fraction of variance, others using the variance itself. If the fraction calculation is embedded in the function, you need to run it twice. My proposal would be to add a function, e.g. |
|
@huard How about a |
|
Meh, I also find it annoying when the number of output changes based on the arguments you pass. It reminds me too much of matlab. ; ) |
…tional_uncertainty function
|
Added a function for fractional uncertainty, but did not make any changes to the functions themselves. The test can reproduce the figure from the notebook. I think we're almost there. |
@Zeitsperre not sure what I am supposed to do in this PR to make the test pass ? |
On inspection, the No further action is required. |
<!-- 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:
- [ ] This PR addresses an already opened issue (for bug fixes /
features)
- This PR fixes #xyz
- [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] HISTORY.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?
* Add `xs.ensembles.get_partition_input` to prepare data from catalog to
be passed to xclim partionning functions
(https://xclim.readthedocs.io/en/stable/api.html#uncertainty-partitioning)
* Now, I am wondering if it would be more xscen to also wrap the xclim
fonction instead only prepping the data? This would allow to add a attrs
`processing_level= partition` to be able to put it in a a catalog.
workflow 1: `xs.get_partition_input` -> `xc.ensembles.lafferty_sriver`
-> `fg.partition`
workflow 2: `xs.partition` (which gets the input and wraps xclim) ->
maybe save to disk and catalog -> `fg.partition` or other plots (maybe
maps of one component)
EDIT: I went with workflow 1
### Does this PR introduce a breaking change?
no
### Other information:
* I wasn't sure if this should be in `extract.py` (because we extract
from a cat) or in `ensembles.py` ( to match the xclim module).
* companion de Ouranosinc/xclim#1529 et
Ouranosinc/figanos#134
Pull Request Checklist:
number) and pull request (:pull:number) has been addedWhat kind of change does this PR introduce?
Does this PR introduce a breaking change?
depends (see options below)
Other information:
TODO list in this PR:
numtohawkins_sutton?TODO list outside of this PR:
graph_fraction_of_total_varianceto figanos (in progress by Juliette, add partition plot figanos#134)In this function, I could rename the dimension to fit what the partition vocabulary (
model,scenario,downscaling) OR we could change the partition vocab to the catalog/xscen vocab (source,experiment,bias_adjust_project). Option 2 is my personal preference, but that would be a breaking change forhawkins_sutton.