Steps blending v1 #255

dnerini · 2022-01-14T07:55:11Z

This PR needs to implement the changes needed to make the blending module compatible with v1 (i.e. without xarray).

* First basic functions to implement STEPS blending * Add compute of blend means,sigmas and recompose * pysteps.io with xarray (#219) * Add xarray dependency * MCH importer returns an xarray Dataset * Remove plot lines * Remove import * Adapt readers to xarray format * Rewrite as more general decorator * Add missing metadata * Adapt io tests * Mrms bounding box (#222) * Fix bounding box coordinates * Add missing metadata * Import xarray DataArray Ignore quality field * Black * Do not hardcode metadata * Address review comments by ruben * Add a legacy option to the io functions A "legacy" options is added to revert back the importers and readers behavior to version 1. This is a temporary solution to allow the examples, and other functions, to run as usual (v1.*). Hopefully, this is will allow a easier transition into version 2 during the development process and will allow testing functions that were not updated to v2. * Fix compatibility problems with tests Many of the tests were updated to use the legacy data structures (v1). The tests that still contains issues, were tagged with a TODO message and they are skipped. This will allow incremental changes to be tested in the new v2. IMPORTANT: once the v2 branch is stable, we may remove the legacy compatibility from the code base and the tests. * Update dependencies * Ignore plugins test Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> * Add blend_optical_flow * changes to steps blending procedure - weights according to adjusted BPS2006 method * changes to blending procedures - adjust weights from original BPS2006 method * Determine spatial correlation of NWP model forecast * First attempt to make correlations and thus weights lead time dependent (in progress..) * Change back to original BPS2006 blending formulation and add regression of skill values to climatological values for weights determination * Reformat code with Black * Skill score script imports climatological correlation-values from file now * Small changes to skill score script * Add skill score tests and an interface * Add skill score tests and an interface * Small change to docstring * Bom import xarray (#228) * Add import_bom_rf3 using xarray * Add tests to xarray version * Fix mrms importer tests * Pass **kwargs to internal functions * Add nwp_importers to read bom nwp sample data * Add bom nwp data to source file * Add tests for bom_nwp reader * Fix pystepsrc Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> * Functions to store and compute climatological weights (#231) * Implement the functions get_default_weights, save_weights, calc_clim_weights. These functions are used to evolve the weights in the scale- and skill-dependent blending with NWP in the STEPS blending algorithm. The current weights, based on the correlations per cascade level, are regressed towards these climatological weights in the course of the forecast. These functions save the current and compute the climatological weights (a running mean of the weights of the past n days, where typically n=30). First daily averages are stored and these are then averaged over the running window of n days. * Add tests for pysteps climatological weight io and calculations. * Add path_workdir to outputs section in pystepsrc file and use it as a default path to store/retrieve blending weights. * Minor changes to docstrings, changes to skill scores and testing scripts * Completed documentation for blending clim module, cleanup. Co-authored-by: RubenImhoff <r.o.imhoff@live.nl> * Main blending module, first steps * Add simple tests * Minor changes to tester: velocity now based on rainfall field of NWP * Add utilities to decompose, store and load NWP cascades for use in blending (#232) * First version of NWP decomposition * Added saving to netCDF * Completed functions for saving and loading decomposed NWP data * Added example files for the decomposed NWP functions * Added compatibility with numpy datetime * Use default output path_workdir for tmp files in blending/utils.py. * Update documentation of NWP decomposition functions in utils.py Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> Co-authored-by: wdewettin <87696913+wdewettin@users.noreply.github.com> * Add importer for RMI NWP data (#234) Add importer for netcdf NWP data from RMI using xarrays. * Add test for RMI NWP data importer. * Add entry for RMI NWP data in pystepsrc. * Run black on everything: fix formatting. * Add KNMI Harmonie NWP netcdf importer and tests (#235) * Changes to v_models to make use of multiple timesteps. Changes in the velocity field over time in the NWP forecast will be taken into account now. * Fixes for KNMI importer: Add forgotten @postprocess_import() Don't call dropna on NWP data. * Avoid shadowing of pysteps.blending.utils by pysteps.utils * First attempt for probability matching and masking utility; part 1 * Changes to prob matching and masking methods; part 2 * Prob matching and masking changes; part 3. Ready for testing with real data from here on * Remove unnecessary print statements * Cleanup imports * More cleanup * Update docstrings * RMI importer for gallery example (will follow) * Reprojection functionality (#236) * Added Lesley's reprojection module to this branch * Added compatibility for three-dimensional xarrays * Add commentary to reprojection util * Changes to make reprojection of KNMI data possible * Changes after Daniele's review * Add dependencies * Changes to importers, see issue #215 * Add tests * Fix some issues * documentation * Fixes for tests * Set requirements again * Some fixes * Changes to nwp_importers after Carlos' response * Remove wrong example script * Remove rasterio dependencies from lists * First try to prevent testing error * Changes Daniele and fix knmi nwp importer * Add rasterio to tox.ini * Aesthetics * rasterio import test * Add rasterio to the test dependencies * Reset try-except functionality for rasterio import * Fix for failing test on windows python 3.6 * add importerskip rasterio Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * Fixes in nwp importers * Revert "Merge branch 'steps_blending' into pysteps-v2" (#239) This reverts commit 2c639f8, reversing changes made to bccb8fc. * Merge latest version pysteps-v2 into steps_blending branch (#237) * Update docstrings * More cleanup * Cleanup imports * Cleanup imports * More cleanup * Update docstrings * Update references Mention the work of Ravuri et al (2021, Nature) as an example of work using cGANs to generate ensembles * Clean up page * Reprojection functionality (#236) * Added Lesley's reprojection module to this branch * Added compatibility for three-dimensional xarrays * Add commentary to reprojection util * Changes to make reprojection of KNMI data possible * Changes after Daniele's review * Add dependencies * Changes to importers, see issue #215 * Add tests * Fix some issues * documentation * Fixes for tests * Set requirements again * Some fixes * Changes to nwp_importers after Carlos' response * Remove wrong example script * Remove rasterio dependencies from lists * First try to prevent testing error * Changes Daniele and fix knmi nwp importer * Add rasterio to tox.ini * Aesthetics * rasterio import test * Add rasterio to the test dependencies * Reset try-except functionality for rasterio import * Fix for failing test on windows python 3.6 * add importerskip rasterio Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * Revert "Merge branch 'steps_blending' into pysteps-v2" (#239) This reverts commit 2c639f8, reversing changes made to bccb8fc. Co-authored-by: ned <daniele.nerini@meteoswiss.ch> Co-authored-by: dnerini <daniele.nerini@gmail.com> Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * NWP skill calculation only within radar domain * Update docs * Add example for gallery examples * Fix docstrings example * Remove additional normalization step * Fixes for the tests * update docs * changes to post-processing rainfall field and docstrings * Update contributing guidelines (#241) - Improve grammar. - Make the guide more concise. Remove unused/unnecessary rules. - Indicate more clearly which parts of the guidelines are inspired by other projects (before they were only mentioned at the end). - Change "Travis-CI" references by "GitHub Actions". * Advect noise cascade * Allow for moving domain mask of extrapolation component * minor fixes * Linear blending (#229) * Implemented linear blending function * Added example file and test * Added compatibility for NWP ensembles The PR is ready to go. Making the code xarray ready will be done in a separate PR. Co-authored-by: RubenImhoff <r.o.imhoff@live.nl> * weights calculation adjustment outside radar domain if only one model present * allow for mirroring of advected noise cascade * implementation of weights following Seed et al. (2013) * Allow for decomposed NWP precip and NWP velocity fields: part 2 * Store decomposed fields with compression * changes after first review Daniele * Remove unnecessary print statement * fixes to blending utils and implementation of blending utils tests * remove unnecessary lines * Fix one time step shift of extrapolation skill prior to blending * minor changes to blending climatology, blending weights and remove path_workdir from pystepsrc * Make NWP forecast decomposition prior to blending function optional * Use pathlib * Extract methods * Minor changes to docstrings * Access climatological skill file for multiple NWP model and date string changes to prevent errors in blending.utils Co-authored-by: Carlos Velasco <carlos.velasco@bom.gov.au> Co-authored-by: ned <daniele.nerini@meteoswiss.ch> Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> Co-authored-by: Ruben Imhoff <Ruben.Imhoff@deltares.nl> Co-authored-by: Carlos Velasco <cvelascof@gmail.com> Co-authored-by: Lesley De Cruz <lesley.decruz+git@gmail.com> Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> Co-authored-by: wdewettin <87696913+wdewettin@users.noreply.github.com> Co-authored-by: Lesley De Cruz <lesley.decruz@meteo.be> Co-authored-by: dnerini <daniele.nerini@gmail.com>

dnerini · 2022-01-14T08:47:06Z

In order to merge into master v1 there are few pending issues that we need to solve:

1. Remove xarray hard dependency

I already started addressing this with b947b6b but some work remains. In particular, the reprojection module is based on xarray and needs to be migrated to numpy. Alternatively, we could make xarray an optional dependency. I also suggest to remove the bom importers based on xarray.
@ladc @RubenImhoff @cvelascof

2. NWP Importers

NWP importers should be separated from the base package as plugins, see #218. This could also a way to address the issue above, since the xarray dependency could be moved to the external plugin. For now, we suggest we do the exercise with the KNMI NWP importer and use that one for testing and examples of blending in the base package. What do you think @RubenImhoff?

RubenImhoff · 2022-01-14T08:53:45Z

I'll have a look at it (in the coming days) too!

codecov · 2022-01-14T10:05:05Z

Codecov Report

Merging #255 (3a2cac0) into master (72f3bf5) will increase coverage by 1.29%.
The diff coverage is 90.15%.

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   80.92%   82.22%   +1.29%     
==========================================
  Files         142      158      +16     
  Lines       10758    12088    +1330     
==========================================
+ Hits         8706     9939    +1233     
- Misses       2052     2149      +97

Flag	Coverage Δ
unit_tests	`82.22% <90.15%> (+1.29%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pysteps/decorators.py	`99.14% <ø> (ø)`
pysteps/tests/helpers.py	`94.28% <ø> (ø)`
pysteps/tests/test_datasets.py	`72.09% <ø> (ø)`
pysteps/tests/test_nowcasts_steps.py	`100.00% <ø> (ø)`
pysteps/tests/test_plugins_support.py	`100.00% <ø> (ø)`
pysteps/tests/test_verification_detcatscores.py	`100.00% <ø> (ø)`
pysteps/tests/test_interfaces.py	`96.62% <28.57%> (-3.38%)`	⬇️
pysteps/blending/interface.py	`50.00% <50.00%> (ø)`
pysteps/blending/linear_blending.py	`80.76% <80.76%> (ø)`
pysteps/blending/steps.py	`83.89% <83.89%> (ø)`
... and 36 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72f3bf5...3a2cac0. Read the comment docs.

RubenImhoff · 2022-01-14T13:55:23Z

@ladc, I see that there is an examples/test_decomposition_NWP.py. Do we want to keep this example? It could be useful, I think, but perhaps we should add some more doc strings and printed output in it.

* Set minimum python version to 3.7 and maximum version to 3.9 * Update github actions Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com>

RubenImhoff · 2022-01-17T09:26:33Z

@dnerini, how should we move forward with the external nwp importer plugins? Do you want to use the cookiecutter implementation or make a separate plugin that basically uses the current nwp_importers.py implementation?

RubenImhoff · 2022-01-17T10:43:36Z

I changed the reprojection script to work without xarray. It (for now) assumes that nwp_importers returns an array with the rainfall fields and a separate metadata dictionary (similar to io.importers.py). So the reprojection tests should pass if these importers return that.

dnerini · 2022-01-17T16:13:40Z

hould we move forward with the external nwp importer plugins? Do you want to use the cookiecutter implementation or make a separate plugin that basically uses the current nwp_importers.py implementation?

yes I think we should give the cookicutter plugin from @aperezhortal a try to implement the nwp importers! No idea if this will work ... can we start with the KNMI importer?

RubenImhoff · 2022-01-18T14:31:11Z

The cookiecutter is a nice tool, @aperezhortal! It took some time to get familiar with it, but I have all three NWP importers in a separate plugin now. A couple of questions to move forward (and let you review it):

Should I upload the plugin as a separate repository in pySTEPS? Then you could review the PR there.
How to link the plugin to pysteps (locally it is easy with the installer, but does this also directly work in a repository)?
For the tests in the plugin, how to link the pysteps-data-master to it (similar to how that is done in pysteps)?

aperezhortal · 2022-01-18T15:15:41Z

Hi @RubenImhoff, you wrote the first-ever known plugin 💪

About your questions:

Should I upload the plugin as a separate repository in pySTEPS? Then you could review the PR there.

That is good idea. You can perhaps upload it to an "initial_commit" branch and then submit a PR to the main branch with an initial commit that is just a README. I will be happy to take a look at it.

How to link the plugin to pysteps (locally it is easy with the installer, but does this also directly work in a repository)?

Do you mean to link the plugin to pysteps repository's workflow to load the NWP test data? If so, I can think of two ways of doing that. One is to add a new step to the CI workflow that installs the plugins after pysteps is installed. This seems like the simpler approach to me. For free, we are also testing the integration of this plugin.
The other option is to avoid any explicit dependency on the plugin and create fixtures for tests in formats that are easy to read (like numpy of h5). The advantage here is that the CI workflow does not depend on the plugin to run the test, which is also good since the latter is not an explicit dependency of the blending module. This will perhaps help to keep things simple.
What do you think ?

For the tests in the plugin, how to link the pysteps-data-master to it (similar to how that is done in pysteps)?

If only a few files are needed, one option is to create a small script that downloads those files before running the tests.

RubenImhoff · 2022-01-18T15:44:42Z

Thanks for your quick response, @aperezhortal! I'll make a PR of the plugin - then we can also discuss further there.

You can perhaps upload it to an "initial_commit" branch and then submit a PR to the main branch with an initial commit that is just a README.

What do you mean with the initial commit just containing a README? You mean just putting the README in there without the rest of the files?

Do you mean to link the plugin to pysteps repository's workflow to load the NWP test data? If so, I can think of two ways of doing that. One is to add a new step to the CI workflow that installs the plugins after pysteps is installed. This seems like the simpler approach to me. For free, we are also testing the integration of this plugin.
The other option is to avoid any explicit dependency on the plugin and create fixtures for tests in formats that are easy to read (like numpy of h5). The advantage here is that the CI workflow does not depend on the plugin to run the test, which is also good since the latter is not an explicit dependency of the blending module. This will perhaps help to keep things simple.
What do you think ?

The first option is easy with the examples script, but I believe no other tests (except for the previous nwp_importers) depend on the presence of the importers. As the examples gallery is being decoupled from the main repository anyway, the latter option might work as well (though the examples gallery repo requires the plugin then..).

If only a few files are needed, one option is to create a small script that downloads those files before running the tests.

Good idea, let's put it in a PR and continue from there.

aperezhortal · 2022-01-18T20:57:36Z

What do you mean with the initial commit just containing a README? You mean just putting the README in there without the rest of the files?

Yes. I mentioned that to make sure that the destination branch of the PR exists and leave it empty to avoid having work-in-progress code in the main branch.

dnerini · 2022-01-27T15:19:51Z

Personally I think this is ready to be merged!

RubenImhoff · 2022-02-03T08:01:26Z

Nice work with the variable names, @dnerini! That was was still somewhere on my to-do list, so great it's done. :)

pysteps/tests/test_utils_reprojection.py

And various fixes to the docs

RubenImhoff · 2022-02-15T09:48:05Z

In the original STEPS paper and technical note (in particular, the BPS2004 technical note), the lag-1 and lag-2 parameters are allowed to regress to their climatological values (eq. 12 - 19). By doing so, the Phi parameters change over time, which enhances the AR process. I have not implemented this (yet) in the steps blending module and saw that it also isn't part of the pysteps steps nowcasting module (right?). Is it worth implementing this, or is it left out of pysteps for a reason?

Other than that, this PR is good to go! :)

dnerini · 2022-02-15T16:47:13Z

In the original STEPS paper and technical note (in particular, the BPS2004 technical note), the lag-1 and lag-2 parameters are allowed to regress to their climatological values (eq. 12 - 19). By doing so, the Phi parameters change over time, which enhances the AR process. I have not implemented this (yet) in the steps blending module and saw that it also isn't part of the pysteps steps nowcasting module (right?). Is it worth implementing this, or is it left out of pysteps for a reason?

Other than that, this PR is good to go! :)

good that you mention it (and perhaps it could be included in the notes section of the method itself), but I'd leave this as "future development" ;-)

RubenImhoff · 2022-02-15T18:00:14Z

As discussed with @dnerini, the PR is ready to be merged. Any further developments can go through separate PRs.

* Re-add option to upscale mrms data * Remove references to xarray

dnerini added this to the v1.6 milestone Jan 14, 2022

First changes to remove xarray dependency

b947b6b

dnerini added 3 commits January 14, 2022 10:22

Do not format json with black

46cb9a0

Comment out xarray code

b6f6ddc

Remove 'legacy' argument

b1f99b3

RubenImhoff assigned ladc, dnerini, cvelascof and RubenImhoff Jan 14, 2022

RubenImhoff and others added 5 commits January 14, 2022 15:13

Fix issues from codacy report - v1

8bb3a0a

Fix issues from codacy report - v2

52aacab

Fix black

dc4ccab

Set minimum python version to 3.7 (#253)

ae3f4d4

* Set minimum python version to 3.7 and maximum version to 3.9 * Update github actions Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com>

Ignore vscode and macos folders

3197383

Change reprojection, test and example script to work without xarray

123388a

dnerini added 2 commits January 17, 2022 17:17

Merge branch 'master' into blending-v1

299df11

Fix test

230737b

RubenImhoff linked an issue Jan 19, 2022 that may be closed by this pull request

Implement STEPS blending scheme #212

Closed

12 tasks

dnerini requested a review from pulkkins January 27, 2022 15:20

dnerini added 7 commits January 28, 2022 09:33

Fix table

6a9f1d4

Plot results as columns

c5f5212

Improve docstrings

7e6bb95

Fix black

b145ef6

Merge branch 'master' into blending-v1

0d63879

Rename (some) variables

99fa108

Merge branch 'master' into blending-v1

0c2b621

RubenImhoff and others added 7 commits February 3, 2022 11:39

Make sure NWP correlations are always finite numbers

0d7f302

Make sure climatological skill is always positive and finite

29e960f

Fix reference in docstrings

0ba919f

List rasterio as optional dependency

d7a4306

Make n_ens_members a mandatory argument

5cad933

Remove default parameters

19df0d2

Fix black

e35c00a

dnerini commented Feb 11, 2022

View reviewed changes

pysteps/tests/test_utils_reprojection.py Show resolved Hide resolved

dnerini added 3 commits February 11, 2022 10:37

Rename reprojection method

587ef13

And various fixes to the docs

Add missing test

642fd75

Use micromamba 0.20

c8e0063

add notes about lag-1 and lag-2 parameter regression

3a2cac0

RubenImhoff approved these changes Feb 15, 2022

View reviewed changes

RubenImhoff merged commit 5890e21 into master Feb 15, 2022

dnerini deleted the blending-v1 branch February 15, 2022 19:50

dnerini mentioned this pull request Feb 18, 2022

Revert some changes from #255 #271

Merged

dnerini added a commit that referenced this pull request Feb 18, 2022

Revert some changes from #255 (#271)

a9e4cc8

* Re-add option to upscale mrms data * Remove references to xarray

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Steps blending v1 #255

Steps blending v1 #255

dnerini commented Jan 14, 2022 •

edited

Loading

dnerini commented Jan 14, 2022 •

edited

Loading

RubenImhoff commented Jan 14, 2022

codecov bot commented Jan 14, 2022 •

edited

Loading

RubenImhoff commented Jan 14, 2022

RubenImhoff commented Jan 17, 2022

RubenImhoff commented Jan 17, 2022

dnerini commented Jan 17, 2022

RubenImhoff commented Jan 18, 2022

aperezhortal commented Jan 18, 2022

RubenImhoff commented Jan 18, 2022

aperezhortal commented Jan 18, 2022

dnerini commented Jan 27, 2022

RubenImhoff commented Feb 3, 2022

RubenImhoff commented Feb 15, 2022 •

edited

Loading

dnerini commented Feb 15, 2022

RubenImhoff commented Feb 15, 2022

Steps blending v1 #255

Steps blending v1 #255

Conversation

dnerini commented Jan 14, 2022 • edited Loading

dnerini commented Jan 14, 2022 • edited Loading

1. Remove xarray hard dependency

2. NWP Importers

RubenImhoff commented Jan 14, 2022

codecov bot commented Jan 14, 2022 • edited Loading

Codecov Report

RubenImhoff commented Jan 14, 2022

RubenImhoff commented Jan 17, 2022

RubenImhoff commented Jan 17, 2022

dnerini commented Jan 17, 2022

RubenImhoff commented Jan 18, 2022

aperezhortal commented Jan 18, 2022

RubenImhoff commented Jan 18, 2022

aperezhortal commented Jan 18, 2022

dnerini commented Jan 27, 2022

RubenImhoff commented Feb 3, 2022

RubenImhoff commented Feb 15, 2022 • edited Loading

dnerini commented Feb 15, 2022

RubenImhoff commented Feb 15, 2022

dnerini commented Jan 14, 2022 •

edited

Loading

dnerini commented Jan 14, 2022 •

edited

Loading

codecov bot commented Jan 14, 2022 •

edited

Loading

RubenImhoff commented Feb 15, 2022 •

edited

Loading