[ENH] MultiplexForecaster compatibility with multivariate, probabilistic and hierarchical forecasting
#2458
Conversation
fkiraly
added
module:forecasting
|
The test is failing by cutoff indices not moving (being multiple times the same thing) in the I am starting to think there is probably some bug in the legacy function Investigating by refactor, here: #2466 |
|
@miraep8, look what you did! |
|
|
|
Nice!! Not that I am rooting for the bugs here, but it is actually quite satisfying (and surprising!) that the test suite proved to be somewhat useful so quickly. 😄 It seems it was the simpler test you suggested that actually caught it! 👍 |
I guess there is a lesson about testing in there somewhere, although I'm not quite sure what it is... |
There was a problem hiding this comment.
Overall think the changes generally make a lot of sense! I think the HeterogeneousEnsembleForecaster class doesn't add too much here, and should be easy to replace without causing any user side issues (ie we aren't losing any "public" classes). I also think using the _Delegate_Forecaster class seems clever, though does make the code a bit more convoluted. There weren't any obvious issues with the implementation that I spotted. Personally one thing that I would request though would be the addition of a test case that ensures that MultiplexForecaster is in fact now compatible with multivariate/probabilistic/hierarchical forecasting. (ie - just testing one should be fine to make sure the tag cloning has desired effect!)
Provides the functionality discussed in #1614, i.e., a `reset` method in `BaseObject` that resets attributes and internal state of an estimator/object to its post-`__init__` state (while keeping parameters), equivalent to overwriting `self` with a `sklearn.clone`. This is motivated by the bug in `MultiplexForecaster` which @miraep8's test suite uncovered in combination with #2458, and the observation that: * boilerplate needs to be copied from `__init__` to `_fit` to fix it, and * that the same issue possibly exists in a number of other composites which are not tested for subsequent `fit` with different parameters. This PR also adds `reset` at the start of `BaseForecaster.fit`, addressing a family of potential bugs where not initializing in `_fit` causes unexpected behaviour in a sequence of calls `__init__`, `set_params`, `fit` (the bug present in the original #2458).
|
FYI @miraep8, I now removed |
Implements a `MultiplexTransformer`. Related to #2670 and #2459. Note this implementation is meant to be very similar to how `MultiplexForecaster` currently works - see #2458 making use of the `_DelegatedTransformer` in #2612. With `MultiplexTransformer` one now in theory has a new way to multiplex over multiple transformers. (Note one could optionally include various transformers with the pre-existing `OptionalPassthrough` but `MultiplexTransformer` should be useful when people want exactly one transformer.
This PR partially resolves #1364 and #2088, it enables
MultiplexForecasterto work with multivariate, hierarchical, and/or probabilistic forecasters.This is achieved by replacing manual loopthrough with:
_DelegatedForecaster(which includes the probabilistic methods and will also pick up any future method that is added)This PR also contains:
_HeterogeneousMetaEstimatordeepinget_paramsthroughout the code baseThis PR depends on:
update_predictin [ENH] refactor_predict_moving_cutoffand bugfix, outerupdate_predict_singleshould be called #2466