Add first version of the reduced-space ensemble Kalman filter method as part of #464 #491
Conversation
|
Fantastic, Martin! I will have a look at it tomorrow (already gave it a go earlier, of course, but I'll make it a more thorough review now). |
| n_lien: int, (n_ens_members/2) | ||
| Minimum number of ensemble members that forecast precipitation for the Lien | ||
| criterion. As standard, half of the ensemble members should forecast precipitation. | ||
| post_prob_matching: bool, (False) |
There was a problem hiding this comment.
This one is currently not used from what I can see. Should we delete it?
There was a problem hiding this comment.
I've merged the argumentes post_prob_matching and iter_prob_matching into a new one named prob_matching.
| iterative_prob_matching: bool, (True) | ||
| Flag to specify whether a probability matching should be applied at each correction | ||
| step. | ||
| inflation_factor_bg: float, (1.0) |
There was a problem hiding this comment.
Could you add a few lines here describing what the inflation factor is used for?
| Inflation factor of the background (NWC) covariance matrix. | ||
| inflation_factor_obs: float, (1.0) | ||
| Inflation factor of the observation (NWP) covariance matrix. | ||
| offset_bg: float, (0.0) |
There was a problem hiding this comment.
Could you add a few lines here describing what the offset is used for?
| Offset of the background (NWC) covariance matrix. | ||
| offset_obs: float, (0.0) | ||
| Offset of the observation (NWP) covariance matrix. | ||
| nwp_hres_eff: float |
There was a problem hiding this comment.
This one is currently not used from what I can see. Should we delete it?
There was a problem hiding this comment.
nwp_hres_eff is now used in ForecastModel to simulate the standard deviation of the smaller scales.
| return X_ana | ||
|
|
||
| def get_covariance_matrix(self, M: np.ndarray, inflation_factor: float, offset: float): | ||
| """ |
There was a problem hiding this comment.
In this function, can you refer to the equation(s) from Nerini et al. (2019) that are used?
|
To do:
|
pysteps/utils/pca.py
Outdated
| """ | ||
|
|
||
| import numpy as np | ||
| from sklearn import decomposition |
There was a problem hiding this comment.
I think this should be handled as an an optional dependency (and hence documented as such), see other examples in the code
There was a problem hiding this comment.
Thanks @dnerini, I've implemented it in this manner.
There was a problem hiding this comment.
Hi again @dnerini,
the tests are still failing, since the PCA from sklearn is necessary for the correction step. I've talked with @RubenImhoff about this issue and he has also no idea how to handle this.
One option could be to compute a pure extrapolation nowcast instead of a combination when there's no sklearn installed. However, that makes no really sense for me.
There was a problem hiding this comment.
We can install sklearn for the tests, but it shouldn't be added to the base dependencies. This means that users that want to use your method need to install
Sklearn too, otherwise they get an error when they try to use the method
There was a problem hiding this comment.
See example here https://github.com/pySTEPS/pysteps/blob/master/pysteps/feature/tstorm.py#L117
which is documented in the installation page https://pysteps.readthedocs.io/en/stable/user_guide/install_pysteps.html
There was a problem hiding this comment.
Test dependencies are specified here
https://github.com/pySTEPS/pysteps/blob/master/ci/ci_test_env.yml
so you can add sklearn there
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #491 +/- ##
==========================================
+ Coverage 83.95% 84.14% +0.19%
==========================================
Files 163 168 +5
Lines 13739 14507 +768
==========================================
+ Hits 11534 12207 +673
- Misses 2205 2300 +95
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Even after adding the test of the PCA, codacy issues that pca is import in pysteps/utils/init.py but never used. Do you have any ideas what the reason could be for this, @dnerini and @RubenImhoff? |
It gives the same issue for the |
|
We can safely ignore those, they shouldn't appear in the next iterations |
|
Ok, thank you @dnerini, |
|
I adjusted the gallery example. Due to the very short NWP forecast horizon in |
|
@m-rempel, looks like we still have some failing pca tests :( |
|
Not the prettiest plot, nor the best example of the reduced-space ensemble Kalman filter blending, but at least this is a sign that it also work on the KNMI ensemble data (30 members). On my laptop, I could only test it with hourly NWP data, so that explains the mismatches so far. I'll try it out with 5-min data next week. :)
|
|
Thank you @RubenImhoff for sharing these first results with other than DWD data! I've now fixed the bug in the PCA test for cases when n_components < n_ens_member. |
Hi @RubenImhoff and @dnerini!
Here's my first version of the implementation of the reduced-space ensemble Kalman filter method. By calling
blending_method = blending.get_method("pca_enkf")One can now compute a combined forecast by pass arrays of observed precipitation fields, the NWP ensemble model forecast, the motion vector field based on observations as well as the timestamps corresponding to the observations and NWP forecast, respectively.
It's not necessary that the temporal resolution of the NWP forecast is equal to the observation. However, one has to adjust the background inflation factor to >1.0 in this case.
There is also a comprehensive set of additional combination keyword arguments like:
Currently, only an AR(1) process within the forecast step is implemented and the respective tests for the method are not implemented.