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

Add analog downscaling prototype #98

Merged
merged 36 commits into from Aug 13, 2021

Conversation

dgergel
Copy link
Member

@dgergel dgergel commented Jul 3, 2021

This PR adds the analog-inspired, quantile-preserving downscaling method as a new service. It is split up into two services, train_aiqpd and adjust_aiqpd and is based on the implementation of QDM. This is intended to supersede spatial disaggregation as the main mode of downscaling in dodola for the time being.

Basic CLI interface is:

dodola train_aiqpd /path/to/coarse_reference.zarr \
/path/to/fine_reference.zarr 
--out /path/to/trained_afs.zarr \
--v "tasmax", 
--k "+"
dodola apply_aiqpd /path/to/biascorrected.zarr \
/path/to/trainedafs/zarr \
-- year 2026
 --v "tasmax" 
--out /path/to/trained_afs.zarr \

closes #73

@dgergel dgergel added the enhancement New feature or request label Jul 3, 2021
@dgergel dgergel requested a review from brews July 3, 2021 05:07
environment.yaml Outdated Show resolved Hide resolved
environment.yaml Show resolved Hide resolved
Copy link
Member

@brews brews left a comment

Choose a reason for hiding this comment

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

This is cool, @dgergel. I'm pumped to see how this scales.

I have a few inline comments and corrections — as you can see.

In addition:

  • Add this change to HISTORY.rst. Cite this PR and include your github username.
  • Could we squeeze in at least one service.py-test to cover new behaviors/services? I think it'd be fine moving/adapting the core.py tests to fill this gap, if you don't want to write something new (because if service.py is tested, then core.py is tested). But I think we need test coverage for those new services.
  • In dodola/tests/test_cli.py could you add train-aiqpd and apply-aiqpd to the test_cli_helpflags test. The IDs for this would be train-aiqpd --help and apply-aiqpd --help, respectively.

dodola/tests/test_core.py Outdated Show resolved Hide resolved
dodola/services.py Outdated Show resolved Hide resolved
dodola/cli.py Outdated Show resolved Hide resolved
dodola/cli.py Outdated Show resolved Hide resolved
dodola/services.py Outdated Show resolved Hide resolved
dodola/services.py Outdated Show resolved Hide resolved
dodola/tests/test_core.py Outdated Show resolved Hide resolved
environment.yaml Show resolved Hide resolved
@dgergel
Copy link
Member Author

dgergel commented Aug 9, 2021

@brews tests are passing, this is ready for another review!

Copy link
Member

@brews brews left a comment

Choose a reason for hiding this comment

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

Thanks! I have a few other quick changes needed! I'll talk with you about setting up tests.

dodola/services.py Outdated Show resolved Hide resolved
environment.yaml Show resolved Hide resolved
@dgergel
Copy link
Member Author

dgergel commented Aug 12, 2021

@brews I updated the raise statement and noted the test changes we talked about in #113 for a follow-up PR. I think this might be ready to merge.

@dgergel
Copy link
Member Author

dgergel commented Aug 12, 2021

@brews spoke too soon. I need to make a couple updates now that we're pulling from my updated branch of xclim with the edge cases fixed.

@brews
Copy link
Member

brews commented Aug 12, 2021

@dgergel Okay! Appreciate the heads up!

@dgergel
Copy link
Member Author

dgergel commented Aug 12, 2021

@brews okay now it's actually ready!

@brews
Copy link
Member

brews commented Aug 13, 2021

For the record it looks like this is running with https://github.com/ClimateImpactLab/xclim@add_analog_downscaling. The commit hash is c5507ba29c19f957270789fd790ffe6c315a3daa.

Copy link
Member

@brews brews left a comment

Choose a reason for hiding this comment

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

This has all the changes I requested. Thanks!

We're going to want to be sure to pin this new xclim to a commit hash or tag before the next release.

@brews brews changed the title Add analog downscaling Add analog downscaling prototype Aug 13, 2021
@brews brews merged commit 85b4981 into ClimateImpactLab:main Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add analog method to downscaling service
2 participants