Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENH][BUG] clean up Detrender and implement correct multivariate behaviour #4053

Merged
merged 9 commits into from Jan 9, 2023

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Jan 3, 2023

This PR improves the Detrender:

  • it fixes [BUG] incorrect multivariate detrending in Detrender #4051 and also simplifies the logic in the Detrender. As the BaseForecasting interface supports loop over hierarchy instances and variables natively, all the case distinctions internally can go, and with it goes the incorrect multivariate implementation.
  • adds support for detrending with hierarchical forecasters
  • adds support for detrending with forecasters that require fh in fit already (this was not possible previously)

As a consequence, the test test_polynomial_detrending had to be updated since the internal mtype of the Detrender changed, from pd.Series based to pd.DataFrame based (requiring a minor change in how objects are being compared).

@fkiraly fkiraly added module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing bugfix Fixes a known bug or removes unintended behavior enhancement Adding new functionality labels Jan 3, 2023
@fkiraly
Copy link
Collaborator Author

fkiraly commented Jan 3, 2023

FYI @mloning, @SveaMeyer13, @KishManani, would appreciate a look as you have worked on the detrender before.

@fkiraly fkiraly merged commit 8fe5096 into main Jan 9, 2023
@fkiraly fkiraly deleted the cleanup-detrender branch January 9, 2023 21:12
klam-data pushed a commit to CodeSmithDSMLProjects/sktime that referenced this pull request Jan 18, 2023
…haviour (sktime#4053)

This PR improves the `Detrender`:
* it fixes sktime#4051 and also simplifies the logic in the `Detrender`. As the `BaseForecasting` interface supports loop over hierarchy instances and variables natively, all the case distinctions internally can go, and with it goes the incorrect multivariate implementation.
* adds support for detrending with hierarchical forecasters
* adds support for detrending with forecasters that require `fh` in `fit` already (this was not possible previously)

As a consequence, the test `test_polynomial_detrending` had to be updated since the internal mtype of the `Detrender` changed, from `pd.Series` based to `pd.DataFrame` based (requiring a minor change in how objects are being compared).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a known bug or removes unintended behavior enhancement Adding new functionality module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] incorrect multivariate detrending in Detrender
2 participants