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

[BUG] MSTL documentation is misleading after 0.24.0 release #5454

Closed
ArturDragunov opened this issue Oct 20, 2023 · 4 comments · Fixed by #5457
Closed

[BUG] MSTL documentation is misleading after 0.24.0 release #5454

ArturDragunov opened this issue Oct 20, 2023 · 4 comments · Fixed by #5457
Labels
bug Something isn't working module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing

Comments

@ArturDragunov
Copy link

Describe the bug

Up until the recent version, MSTL had a note that it can't inverse. The same note is still present in STLTransformer: STLTransformer can not inverse_transform on indices not seen in fit(). This means that for pipelining, the Deseasonalizer or Detrender must be used instead of STLTransformer. This creates a misleading situation for the users who could have assumed that MSTL can inverse_transform.
To Reproduce

from sktime.datasets import load_airline
import pandas as pd
from sktime.forecasting.naive import NaiveForecaster
from sktime.forecasting.compose import TransformedTargetForecaster
from sktime.transformations.series.detrend.mstl import MSTL
from sktime.transformations.series.impute import Imputer
from sktime.forecasting.model_evaluation import evaluate
from sktime.forecasting.model_selection import ExpandingWindowSplitter


y = load_airline()
y.index = pd.PeriodIndex(y.index, freq="d")
data = pd.DataFrame(y)

for i in range(1, 4):
    data['Lag_' + str(i)] = data[y.name].shift(i)
data.dropna(inplace=True)
    
X = data.drop(columns=y.name)
y = data[y.name]

pipe = TransformedTargetForecaster(steps=[
    ("imputer", Imputer(method="mean")),
    ("deseasonalizer", MSTL(periods=[7])),
    ("forecaster", NaiveForecaster(strategy="drift")),
])

cv = ExpandingWindowSplitter(initial_window=15, step_length=6, fh=[1, 2, 3])
results = evaluate(forecaster=pipe, X=X, y=y, cv=cv) #, error_score='raise')
results

FitFailedWarning:
In evaluate, fitting of forecaster TransformedTargetForecaster failed,
you can set error_score='raise' in evaluate to see
the exception message. Fit failed for len(y_train)=15.
The score will be set to nan.
Failed forecaster with parameters: TransformedTargetForecaster(steps=[('imputer', Imputer(method='mean')),
('deseasonalizer', MSTL(periods=[7])),
('forecaster',
NaiveForecaster(strategy='drift'))]).

result = _evaluate_window(

test_MeanAbsolutePercentageError fit_time pred_time len_train_window cutoff
NaN 0.018867 0.009660 15 NaT
NaN 0.015766 0.007032 21 NaT
NaN 0.016762 0.010788 27 NaT
NaN 0.014447 0.007028 33 NaT

Expected behavior

Ideally, MSTL should have an inverse_transform feature available. Otherwise, the documentation description should change.

Versions
sktime 0.24.0

@ArturDragunov ArturDragunov added the bug Something isn't working label Oct 20, 2023
@fkiraly fkiraly added the module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing label Oct 20, 2023
@fkiraly
Copy link
Collaborator

fkiraly commented Oct 20, 2023

Hm, both STL and MSTL should be able to inverse transform in theory.

I know the feature was added for STL in one of the recent versions so the STLTransformer docstring should be updated at least.

@ArturDragunov
Copy link
Author

@fkiraly I have a small typo in the issue. I was playing with different versions and forgot to change from 0.23.0 to 0.24.0. The error logs have changed but the output is still the same that MSTL can not inverse_transform on indices not seen in fit(). I will see the changes in the source code for STLTransformer. Will experiment with it.

FitFailedWarning:
In evaluate, fitting of forecaster TransformedTargetForecaster failed,
you can set error_score='raise' in evaluate to see
the exception message.
Fit failed for the 0-th data split, on training data y_train with
cutoff NaT, and len(y_train)=15.
The score will be set to nan.
Failed forecaster with parameters: TransformedTargetForecaster(steps=[('imputer', Imputer(method='mean')),
('deseasonalizer', MSTL(periods=[7])),
('forecaster',
NaiveForecaster(strategy='drift'))]).

System:
python: 3.11.4 (main, Jul 5 2023, 14:15:25) [GCC 11.2.0]
machine: Linux-6.2.0-34-generic-x86_64-with-glibc2.37

Python dependencies:
pip: 23.1.2
sktime: 0.24.0
sklearn: 1.3.0
skbase: 0.6.0
numpy: 1.25.2
scipy: 1.11.1
pandas: 2.0.3
matplotlib: 3.7.1
joblib: 1.3.0
numba: None
statsmodels: 0.14.0
pmdarima: None
statsforecast: None

@fkiraly
Copy link
Collaborator

fkiraly commented Oct 20, 2023

I updated the docstring for #5455.

I looked at MSTL, my intention is just to replicate the STL inverse_transform, then it can be used in a transformer pipeline (as a decomposition transformer). The docstring and usage would then be the same.

@fkiraly
Copy link
Collaborator

fkiraly commented Oct 20, 2023

Update for MSTL is here: #5457

I also noticed that the decomposition does not give individual components, I opened an issue here #5458 to track that we should add this as an option for use in a transformed target forecasting pipeline.

fkiraly added a commit that referenced this issue Oct 26, 2023
This PR adds an inverse transform for the `MSTL` transformer, in line
with the `STLTransformer`, which sums up any components from the
decomposition.

Also updates the documentation, and in the process fixes
#5454.

Regarding deprecation, the behaviour should be identical in all cases
where no bug was present, i.e., identical indices seen in `fit` and
`transform`.
fkiraly added a commit that referenced this issue Nov 3, 2023
…nverse and pipelines (#5455)

This PR updates the docstring of `STLTransformer` to correctly account
for statements on inverse and pipelines.

See also discussion in #5454
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants