Closed wdewettin closed 2 years ago
Great work, @wdewettin! I was (actually, we were - that is including @wdewettin and @ladc) wondering whether we should place this method in a/the blending folder or whether it fits in the nowcasts folder as currently done by @wdewettin. Any thoughts?
Great work, @wdewettin! I was (actually, we were - that is including @wdewettin and @ladc) wondering whether we should place this method in a/the blending folder or whether it fits in the nowcasts folder as currently done by @wdewettin. Any thoughts?
Because of the way linear blending works, I personally think it could be provided as a simple postprocessing method (therefore within the postprocessing module) that takes as inputs two separate nowcasts and the relatives weights.
This said, I like the way @wdewettin implemented it, where one has to specify the the nowcasting method and pass the corresponding NWP forecast. But because of this dependency on the actual nowcasting methods, it could make more sense to move into a new blending module. We could say that as a general (and not necessarily very strict) rule, the methods included in a module should be independent one to the other. In this sense, the nowcating module should only include alternative nowcasting methods.
Very nice contribution @wdewettin ! I highly appreciate the fact that you already included all the documentation (including a gallery example!) and tests, well done!
I plan to go through your PR in detail as soon as possible. For now, I have a more general request. Would it be possible to make your method work with an xarray DataArray object instead of numpy arrays?
Yes, @dnerini, I will take a look at making my method work with xarrays!
@dnerini Sorry for my late response! I looked into making it work with xarrays instead of numpy arrays and I found that it would be better to wait with that for the time being. All the functions on which the linear blending function is based still output numpy arrays, and it will be a lot easier to make the linear blending function work with xarrays if those functions are changed first.
:exclamation: No coverage uploaded for pull request base (
pysteps-v2@3802f8d
). Click here to learn what that means. The diff coverage isn/a
.
@@ Coverage Diff @@
## pysteps-v2 #229 +/- ##
=============================================
Coverage ? 73.83%
=============================================
Files ? 152
Lines ? 11213
Branches ? 0
=============================================
Hits ? 8279
Misses ? 2934
Partials ? 0
Flag | Coverage Δ | |
---|---|---|
unit_tests | 73.83% <0.00%> (?) |
Flags with carried forward coverage won't be shown. Click here to find out 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 3802f8d...758f88b. Read the comment docs.
@RubenImhoff @wdewettin @dnerini apologies for the mess in my comments ... not sure what did it go wrong?
All requested changes implemented. Main task left is to make the code xarray ready for pysteps-v2. As discussed before, we may also do that in a separate PR.
This addresses issue #226. I added a nowcasting method called linear blending. This works for all nowcast methods (deterministic and ensemble) and is also compatible with NWP ensembles. Furthermore, I added an example which uses some dummy NWP data to blend with and which plots the results. Lastly, I also added a test for the linear blending function.