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] in BaseForecaster, move check for capability:insample to _check_fh boilerplate #6593

Merged
merged 12 commits into from
Jun 25, 2024
34 changes: 26 additions & 8 deletions sktime/forecasting/base/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ def predict_quantiles(self, fh=None, X=None, alpha=None):
# input checks and conversions

# check fh and coerce to ForecastingHorizon, if not already passed in fit
fh = self._check_fh(fh)
fh = self._check_fh(fh, pred_int=True)

# default alpha
if alpha is None:
Expand Down Expand Up @@ -702,7 +702,7 @@ def predict_interval(self, fh=None, X=None, coverage=0.90):
# input checks and conversions

# check fh and coerce to ForecastingHorizon, if not already passed in fit
fh = self._check_fh(fh)
fh = self._check_fh(fh, pred_int=True)

# check alpha and coerce to list
coverage = check_alpha(coverage, name="coverage")
Expand Down Expand Up @@ -789,7 +789,7 @@ def predict_var(self, fh=None, X=None, cov=False):
# input checks and conversions

# check fh and coerce to ForecastingHorizon, if not already passed in fit
fh = self._check_fh(fh)
fh = self._check_fh(fh, pred_int=True)

# check and convert X
X_inner = self._check_X(X=X)
Expand Down Expand Up @@ -861,7 +861,7 @@ def predict_proba(self, fh=None, X=None, marginal=True):
# input checks and conversions

# check fh and coerce to ForecastingHorizon, if not already passed in fit
fh = self._check_fh(fh)
fh = self._check_fh(fh, pred_int=True)

# check and convert X
X_inner = self._check_X(X=X)
Expand Down Expand Up @@ -1172,16 +1172,16 @@ def update_predict_single(

self.check_is_fitted()

# check fh and coerce to ForecastingHorizon, if not already passed in fit
fh = self._check_fh(fh)

# input checks and minor coercions on X, y
X_inner, y_inner = self._check_X_y(X=X, y=y)

# update internal _X/_y with the new X/y
# this also updates cutoff from y
self._update_y_X(y_inner, X_inner)

# check fh and coerce to ForecastingHorizon, if not already passed in fit
fh = self._check_fh(fh)

# checks and conversions complete, pass to inner update_predict_single
if not self._is_vectorized:
y_pred = self._update_predict_single(
Expand Down Expand Up @@ -1771,7 +1771,7 @@ def fh(self):

return self._fh

def _check_fh(self, fh):
def _check_fh(self, fh, pred_int=False):
"""Check, set and update the forecasting horizon.

Called from all methods where fh can be passed:
Expand All @@ -1791,6 +1791,7 @@ def _check_fh(self, fh):
Parameters
----------
fh : None, int, list, np.ndarray or ForecastingHorizon
pred_int: Check pred_int:insample tag instead of insample tag.

Returns
-------
Expand Down Expand Up @@ -1878,6 +1879,23 @@ def _check_fh(self, fh):
"horizon, please re-fit the forecaster. " + msg
)
# if existing one and new match, ignore new one
in_sample_pred = (
self.get_tag("capability:insample")
if not pred_int
else self.get_tag("capability:pred_int:insample")
)
if (
not in_sample_pred
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these conditions all needed?

E.g., is there an example in which we do know fh but do not know the cutoff?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure so I put them all. I tested it without cutoff, it worked. cutoff is removed now.

and self._fh is not None
and not self._fh.is_all_out_of_sample(self._cutoff)
):
msg = (
f"{self.__class__.__name__} "
f"can not perform in-sample prediction. "
f"Found fh with in sample index: "
f"{fh}"
)
raise NotImplementedError(msg)

return self._fh

Expand Down
3 changes: 0 additions & 3 deletions sktime/forecasting/base/adapters/_neuralforecast.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,6 @@ def _fit(
ValueError
When ``freq="auto"`` and cannot be interpreted from ``ForecastingHorizon``
"""
if not fh.is_all_out_of_sample(cutoff=self.cutoff):
raise NotImplementedError("in-sample prediction is currently not supported")

# A. freq is given {use this}
# B. freq is auto
# B1. freq is infered from fh {use this}
Expand Down
3 changes: 0 additions & 3 deletions sktime/forecasting/base/adapters/_pytorch.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,6 @@ def _predict(self, X=None, fh=None):
fh = self.fh
fh = fh.to_relative(self.cutoff)

if min(fh._values) < 0:
raise NotImplementedError("LTSF is not supporting insample predictions.")

if max(fh._values) > self.network.pred_len or min(fh._values) < 0:
raise ValueError(
f"fh of {fh} passed to {self.__class__.__name__} is not "
Expand Down
14 changes: 1 addition & 13 deletions sktime/forecasting/compose/_reduce.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,7 @@ def _predict_in_sample(self, fh, X=None, **kwargs):
# Note that we currently only support out-of-sample predictions. For the
# direct and multioutput strategy, we need to check this already during fit,
# as the fh is required for fitting.
raise NotImplementedError(
f"Generating in-sample predictions is not yet "
f"implemented for {self.__class__.__name__}."
)
pass

@classmethod
def get_test_params(cls, parameter_set="default"):
Expand Down Expand Up @@ -578,9 +575,6 @@ def _fit(self, y, X, fh):
+ "observation size"
)

if not self.fh.is_all_out_of_sample(self.cutoff):
raise NotImplementedError("In-sample predictions are not implemented.")

yt, Xt = self._transform(y, X)
if hasattr(Xt, "columns"):
Xt.columns = Xt.columns.astype(str)
Expand Down Expand Up @@ -783,9 +777,6 @@ def _fit(self, y, X, fh):
# We currently only support out-of-sample predictions. For the direct
# strategy, we need to check this at the beginning of fit, as the fh is
# required for fitting.
if not self.fh.is_all_out_of_sample(self.cutoff):
raise NotImplementedError("In-sample predictions are not implemented.")

self.window_length_ = check_window_length(
self.window_length, n_timepoints=len(y)
)
Expand Down Expand Up @@ -1125,9 +1116,6 @@ def _fit(self, y, X, fh):
if X is not None:
X = None

if len(self.fh.to_in_sample(self.cutoff)) > 0:
raise NotImplementedError("In-sample predictions are not implemented")

self.window_length_ = check_window_length(
self.window_length, n_timepoints=len(y)
)
Expand Down
5 changes: 0 additions & 5 deletions sktime/forecasting/hf_transformers_forecaster.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,6 @@ def _predict(self, fh, X=None):
fh = self.fh
fh = fh.to_relative(self.cutoff)

if min(fh._values) < 0:
raise NotImplementedError(
"The huggingface adapter is not supporting insample predictions."
)

self.model.eval()
from torch import from_numpy

Expand Down
17 changes: 14 additions & 3 deletions sktime/forecasting/tests/test_all_forecasters.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,13 @@ def test_predict_time_index(

# if estimator cannot forecast in-sample and fh is in-sample, terminate
# if the tag correctly states this, we consider this fine as per contract
# todo: check that estimator raises error message when fitting instead
# check that estimator raises error message when fitting
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use with pytest.raises instead, that is the idiomatic check for an exception being raised. Also check that the error message agrees.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if not fh_is_oos and not estimator_instance.get_tag("capability:insample"):
with pytest.raises(
NotImplementedError,
match="can not perform in-sample prediction",
):
estimator_instance.fit(y_train, fh=fh)
return None

estimator_instance.fit(y_train, fh=fh)
Expand All @@ -245,10 +250,16 @@ def test_predict_time_index(

# if cannot forecast in-sample probabilistically, and fh is in-sample, terminate
# if the tag correctly states this, we consider this fine as per contract
# todo: check that estimator raises error message when fitting instead
# check that estimator raises error message when fitting
if not fh_is_oos:
if not estimator_instance.get_tag("capability:pred_int:insample"):
return None
if estimator_instance.get_tag("capability:pred_int"):
with pytest.raises(
NotImplementedError,
match="can not perform in-sample prediction",
):
estimator_instance.predict_interval()
return None

if estimator_instance.get_tag("capability:pred_int"):
y_pred_int = estimator_instance.predict_interval()
Expand Down
4 changes: 1 addition & 3 deletions sktime/forecasting/tests/test_neuralforecast.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ def test_neural_forecast_univariate_y_without_X(model_class) -> None:
model = model_class(freq="A-DEC", max_steps=5, trainer_kwargs={"logger": False})

# attempt fit with negative fh
with pytest.raises(
NotImplementedError, match="in-sample prediction is currently not supported"
):
with pytest.raises(NotImplementedError):
model.fit(y_train, fh=[-2, -1, 0, 1, 2])

# train model
Expand Down
Loading