Repository navigation
Conversation
RubenImhoff
left a comment
There was a problem hiding this comment.
Hi @alanseed, super great work! Still working on fully understanding all the steps that take place in this parametric approach (also to understand how to use and adapt this in the future). I have not given it a full review yet, but added some notes and suggestions based on my own quick reading through it to understand the method.
It may be good if @ladc, @dnerini and perhaps @pulkkins also have a look at it. :)
There was a problem hiding this comment.
I am very happy with all the documentation attached! This kind of documentation is not present / not the same for the other pysteps modules. How should we proceed? Does it make sense to merge this documentation into one or two docs in a sub folder under either pysteps_reference or user_guide? @dnerini and @ladc, what is your take on this?
There was a problem hiding this comment.
I see that there is quite some documentation already - so then the question remains what we want to do with the documentation under proposals.
| @@ -7,5 +7,6 @@ Implementation of deterministic and ensemble nowcasting methods. | |||
|
|
|||
| .. automodule:: pysteps.noise.interface | |||
| .. automodule:: pysteps.noise.fftgenerators | |||
There was a problem hiding this comment.
| .. automodule:: pysteps.noise.fftgenerators |
| @@ -7,5 +7,6 @@ Implementation of deterministic and ensemble nowcasting methods. | |||
|
|
|||
| .. automodule:: pysteps.noise.interface | |||
There was a problem hiding this comment.
| .. automodule:: pysteps.noise.interface | |
| .. automodule:: pysteps.noise.fftgenerators | |
| .. automodule:: pysteps.noise.interface |
Good reason to directly make this alphabetical. :)
|
|
||
| .. automodule:: pysteps.noise.interface | ||
| .. automodule:: pysteps.noise.fftgenerators | ||
| .. automodule:: pysteps.noise.parametric |
There was a problem hiding this comment.
| .. automodule:: pysteps.noise.parametric |
| .. automodule:: pysteps.noise.fftgenerators | ||
| .. automodule:: pysteps.noise.parametric | ||
| .. automodule:: pysteps.noise.motion | ||
| .. automodule:: pysteps.noise.utils |
There was a problem hiding this comment.
| .. automodule:: pysteps.noise.utils | |
| .. automodule:: pysteps.noise.parametric | |
| .. automodule:: pysteps.noise.utils |
There was a problem hiding this comment.
Looks good! It might be good to add some docstrings to different parts in the functions to ensure all steps are clear for future developers and maintainers.
There was a problem hiding this comment.
This also holds for the code in the functions under ./nowcasts
| The time step of the output is taken from the inputs. | ||
| The ``parametric`` method requires exactly three complete, untransformed | ||
| rain-rate observations in mm/h on a regular grid with square pixels. Pixel | ||
| size and cadence are derived from coordinates; the time stepsize is in seconds. |
There was a problem hiding this comment.
| size and cadence are derived from coordinates; the time stepsize is in seconds. | |
| size and time step are derived from coordinates; the time step is in seconds. |
| or (ens_number, time, y, x) for ensemble methods. Method-specific forecast | ||
| functions document their input history and optional settings. | ||
|
|
||
| The output depends on the type of the method. |
There was a problem hiding this comment.
Good to still replace this with what the output is (an xarray DataSet)
| return selected, audit | ||
|
|
||
|
|
||
| def _advance( |
There was a problem hiding this comment.
Good to add some docstrings to the function.
| from ._cascade_advection import advect_field, prepare_advection_coordinates | ||
|
|
||
| # Evaluated nowcast_v2 settings, not the later plotting-only timescale fit. | ||
| _ESTIMATION_DEFAULTS = dict( |
There was a problem hiding this comment.
Are these all default settings that the user can change? And if so, does this happen in the main calling function? If that is the case, I would prefer having the defaults set in the function (as default) and then these dictionaries can be filled with the variable value (either the default or the user-assigned value).
RubenImhoff
left a comment
There was a problem hiding this comment.
Hi @alanseed, super great work! Still working on fully understanding all the steps that take place in this parametric approach (also to understand how to use and adapt this in the future). I have not given it a full review yet, but added some notes and suggestions based on my own quick reading through it to understand the method.
It may be good if @ladc, @dnerini and perhaps @pulkkins also have a look at it. :)
Adds parametric ensemble nowcasting from three consecutive rain-rate observations, together with calibrated stochastic rainfall generation and marginal, spectral, and temporal estimators. This ports the contribution in #578 to the v2
mainbranch and its xarray dataset interface.The forecast derives grid spacing and cadence from coordinates, accepts supplied motion or estimates Lucas–Kanade motion internally, and returns rain rates with ensemble identifiers, valid times, forecast reference time, and geographic metadata. It preserves the NumPy numerical engine, independent member random streams, temporal-only fallback, and the distinction between missing and dry input. Callbacks remain arrays and support generation without retaining the full ensemble.
The synthetic and Swiss examples use the dataset interface. The Swiss example includes lossless native NetCDF export and a GIF of three QPE observations followed by the first member's six forecasts.
Validation:
Scope: the first interface supports complete regular grids with square pixels and integer lead counts. These checks establish implementation and serialization behaviour; they do not establish Swiss calibration or forecast skill. CF support for additional projections, including UTM and Swiss somerc, remains separate. Existing projection text and available latitude/longitude coordinates are preserved.
Cross-platform and Python 3.11 validation will run in upstream CI.