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.

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

sdba - Fix DQM and QDM, refactor adjustment objects #447

Merged
merged 12 commits into from
May 13, 2020
Merged

Conversation

aulemahal
Copy link
Collaborator

@aulemahal aulemahal commented May 12, 2020

Pull Request Checklist:

  • What kind of change does this PR introduce?

Fixes #441 by adding a grouped normalization before the detrending in DQM.

  • Does this PR introduce a breaking change?
    No, but the behavior is not exactly what a user could expect from reading Cannon et al. 2015, a note is added in the docs to clarify this.

  • Other information:

@aulemahal aulemahal changed the title Sdba fix dqm 441 sdba - Fix DQM by grouped normalization May 12, 2020
@aulemahal aulemahal requested a review from huard May 12, 2020 17:25
@aulemahal aulemahal self-assigned this May 12, 2020
@aulemahal aulemahal added this to the v0.17 milestone May 12, 2020
Copy link
Collaborator

@huard huard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has a test already been added for this in the other PRs ? Otherwise we'll want one.

@coveralls
Copy link

coveralls commented May 12, 2020

Pull Request Test Coverage Report for Build 1864

  • 0 of 39 (0.0%) changed or added relevant lines in 2 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.4%) to 76.384%

Changes Missing Coverage Covered Lines Changed/Added Lines %
xclim/sdba/utils.py 0 10 0.0%
xclim/sdba/adjustment.py 0 29 0.0%
Files with Coverage Reduction New Missed Lines %
xclim/sdba/utils.py 1 0%
xclim/sdba/adjustment.py 4 0%
Totals Coverage Status
Change from base Build 1861: -0.4%
Covered Lines: 2484
Relevant Lines: 3252

💛 - Coveralls

@aulemahal aulemahal changed the title sdba - Fix DQM by grouped normalization sdba - Fix DQM and QDM, refactor adjustment objects May 13, 2020
@aulemahal
Copy link
Collaborator Author

Merged fixes for QDM in this PR to save time. Dask-arrays needed a new version of rank so I replicated the xarray one but with an apply_ufunc that allows for dask parallelization.

@aulemahal aulemahal requested a review from huard May 13, 2020 17:20
@aulemahal
Copy link
Collaborator Author

My bad, the fix to xclim.sdba.utils.broadcast did not fix #449 for all cases, so I am reverting it for now.
Also, the new rank util partially fixes #448. However, it is not a great use as long as 449 is not fixed...

@@ -37,13 +37,17 @@
- preprocessing on `ref`, `hist` and `sim` (using methods in `xclim.sdba.processing` or `xclim.sdba.detrending`)
- creating the adjustment object `Adj = Adjustment(**kwargs)` (from `xclim.sdba.adjustment`)
- training `Adj.train(obs, sim)`
- adjustment `scen = Adj.adjust(sim)`
- adjustment with corresponding arguments `scen = Adj.adjust(sim, **kwargs)`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- adjustment with corresponding arguments `scen = Adj.adjust(sim, **kwargs)`
- adjustment `scen = Adj.adjust(sim, **kwargs)`

- post-processing on `scen` (for example: re-trending)

The train-adjust approach allows to inspect the trained adjustment object. The adjustment information is stored in
The train-adjust approach allows to inspect the trained adjustment object. The trained information is stored in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The train-adjust approach allows to inspect the trained adjustment object. The trained information is stored in
The train-adjust approach allows to inspect the trained adjustment object. The training information is stored in

@@ -127,17 +132,22 @@ class EmpiricalQuantileMapping(BaseAdjustment):

Parameters
----------
At init:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
At init:
At instantiation:

group : Union[str, Grouper]
The grouping information. See :py:class:`xclim.sdba.base.Grouper` for details.

In adjust:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In adjust:
In adjustment:

@aulemahal aulemahal merged commit 6393a9b into master May 13, 2020
@aulemahal aulemahal deleted the sdba-fix-dqm-441 branch May 13, 2020 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DetrendedQuantileMapping only applies the last correction factor
4 participants