Spatial morphing - #2441
Spatial morphing#2441
Conversation
…ransition point between forecast sources.
…ight precipitation.
…ons sources is absent.
mo-jbeaver
left a comment
There was a problem hiding this comment.
I've added some comments below, majority are docstring related. The unit tests passed successfully but the acceptance tests failed with assertion errors
| nearest_fp, use_secondary, secondary_map, primary_map, cluster_cube | ||
| ) | ||
| if self.cluster_number not in full_cluster_to_selection: | ||
| raise ValueError( |
There was a problem hiding this comment.
Value Error should possibly be added to the docstring
bayliffe
left a comment
There was a problem hiding this comment.
Tiny comments, nearly there.
| for template_slice in template_slices | ||
| ] | ||
| elif np.isscalar(self.interpolation_fractions): | ||
| fractions = [float(self.interpolation_fractions)] * len(template_slices) |
There was a problem hiding this comment.
I remain confused. Why do we have multiple template slices if we are simply morphing between two input fields at a fixed validity time?
| If supplied, cube1 and cube2 may have the same validity time, | ||
| allowing source morphing at a fixed validity time. A scalar applies | ||
| the same fraction to every output slice; a sequence provides one | ||
| fraction per output slice. |
There was a problem hiding this comment.
I think I'm confused again (along with the query about multiple template_slices previously) do we ever morph to multiple steps between two fields valid at the same time?
There was a problem hiding this comment.
I've modified this text. From the SpatialMorphing plugin, we only ever take two inputs and morph to a single intermediate point. We never morph to multiple points. Just to clarify, the GoogleFilmInterpolation plugin could morph to multiple points, if we wanted to support it, but the GoogleFILMInterpolation plugin is never called in that way from the SpatialMorphing plugin. I think that the confusion arises because the GoogleFILMInterpolation plugin could do things in a more general way than we want to support specifically for the SpatialMorphing plugin.
…learly within the GoogleFILMInterpolation changes.
|
I've now added these two commits: c8381f8 and e4dbf6d. These are all related to this comment: #2441 (comment). I previously tried to address this by making the the selection of the bounding pair of forecast sources cluster-aware, but after testing, I realised that this only worked for "nc_det uk_det" => "uk_det" and "nc_det uk_det" => "uk_ens" and wasn't sufficient for a cluster defined with "nc_det uk_det" => "uk_det" => "uk_ens", because "nc_det uk_det", "uk_det", "uk_ens" were all present in one cluster, so the selection of the bounding pair of forecast sources also needed to be forecast_period aware. |
|
The latest commit (bd2c3b0), I think relates to changes made in relation to this comment: #2441 (comment), where I removed |
bayliffe
left a comment
There was a problem hiding this comment.
Thanks for the changes Gavin. Happy to leave this alone now...
…sion * upstream/master: Spatial morphing (metoppv#2441) Stochastic noise extensions (metoppv#2442) Forecast capping if calibrate forecast is too far from the uncalibrated (metoppv#2449) Vicinity metadata from the threshold plugin (metoppv#2439) Update cluster sources bookkeeping, so that forecast periods are only stored in one forecast source. (metoppv#2437) Correct realization numbering bug in temporal interpolation (metoppv#2436) Modify precedence ordering when clustering (metoppv#2435)
* master: Spatial Morphing - refinement of Google FILM output (#2451) Spatial morphing (#2441) Stochastic noise extensions (#2442) Forecast capping if calibrate forecast is too far from the uncalibrated (#2449) Vicinity metadata from the threshold plugin (#2439) Update cluster sources bookkeeping, so that forecast periods are only stored in one forecast source. (#2437)
Related to https://github.com/metoppv/mo-blue-team/issues/1268
improver_test_data PR: metoppv/improver_test_data#146
Description
This PR seeks to address weaknesses with the current use of temporal interpolation as implemented using the ForecastTrajectoryGapFiller plugin. The main weaknesses are:
The SpatialMorphing plugin effectively combines two existing plugins i.e.
RealizationSelectionandForecastTrajectoryGapFiller, as this plugin itself takes the clustering cube as an input, so that it can select the relevant realizations to use as input to Google FILM.Testing: