Modifications following review comments #17
Conversation
…tures from the config and the calling of prep_features to reduce for loops.
…ifier for training and application.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## mobt_877_implement_qrf #17 +/- ##
=========================================================
Coverage ? 95.71%
=========================================================
Files ? 142
Lines ? 14158
Branches ? 0
=========================================================
Hits ? 13551
Misses ? 607
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| to draw from the total number of samples available to train each tree. | ||
| If None, then each tree contains the same number of samples as the total | ||
| available. The trees will therefore only differ due to the use of | ||
| bootstrapping (i.e. sampling with replacement) when creating the |
| Returns: | ||
| None: | ||
| The function creates a pickle file. | ||
| A quantile regression random forest model. |
There was a problem hiding this comment.
Should we not still call it a pickle file so the user can identify the file type they're looking for?
There was a problem hiding this comment.
I've extended this docstring.
|
|
||
| class LoadAndApplyQRF(PostProcessingPlugin): | ||
| class PrepareAndApplyQRF(PostProcessingPlugin): | ||
| """Load and apply the trained Quantile Regression Random Forest (QRF) model.""" |
There was a problem hiding this comment.
This docstring wants updating now we've changed the name
| cube_inputs: List of cubes containing the features and the forecast to be | ||
| calibrated. | ||
| qrf_descriptors: The trained QRF model to be applied to the forecast | ||
| and the transformation and pre-transform addition applied during |
There was a problem hiding this comment.
I think this should include what's in the comment below about what's expected, e.g. Descriptors expected: (qrf_model, transformation, pre_transform_addition)
| unique_site_id_keys: Union[list[str], str] = "wmo_id", | ||
| ): | ||
| """Initialise the LoadAndTrainQRF plugin.""" | ||
| """Initialise the LoadForQRF plugin. |
| pre_transform_addition (float): | ||
| Value to be added before transformation. | ||
| calibrated e.g. air_temperature. This will be used to separate it from | ||
| the rest of the feature cubes, if present. |
There was a problem hiding this comment.
I think this is missing the name of the argument: unique_site_id_keys (list):
There was a problem hiding this comment.
Added missing argument to docstring.
| raise IOError(err) | ||
| # if recreate_file_path == kgo_path: | ||
| # err = ( | ||
| # f"Recreate KGO path {recreate_file_path} must be different from" |
There was a problem hiding this comment.
Is this something that is intended to be committed?
3 participants
Addresses metoppv#2135
Description
This PR includes modifications following the review comments in metoppv#2135. Some additional changes have also been made, for example, adding support for dynamic features and the use of a coordinate other than
wmo_idto uniquely identify each site.Note that to help with avoiding duplication, the split_pickle_parquet_and_netcdf and identify_parquet_type functions, and the pickle file handling from
improver/cli/__init__.pyhas been copied metoppv#2153.Testing: