Skip to content

Fix bug in new NNs with forecasting interval #1108

Merged
merged 9 commits into from
Feb 28, 2023
Merged

Fix bug in new NNs with forecasting interval #1108

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f043a54
Fix bug in new NNs with forecasting interval
Feb 10, 2023
dd4c630
Update changelog
Feb 10, 2023
77530be
Merge branch 'inference-v2.1' into issue-1087
Feb 21, 2023
ebbabe2
Mark test test_forecast_out_sample_suffix_failed_rnn
Feb 21, 2023
de7ffbd
Fix error message for seasonal-ma, deadline-ma if context isn't big e…
Feb 21, 2023
b87699f
Add check on NaNs for MLP and tests for it
Feb 21, 2023
38d3936
Refactor validation of batch in MLPNet
Feb 21, 2023
da0d5a8
Update changelog
Feb 21, 2023
af666cb
Update changelog
Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
-
-
### Fixed
-
- Fix alignment during forecasting in new NNs ([#1108](https://github.com/tinkoff-ai/etna/pull/1108))
- Fix `MeanSegmentEncoderTransform` to work with subset of segments and raise error on new segments ([#1104](https://github.com/tinkoff-ai/etna/pull/1104))
-
- Fix `SegmentEncoderTransform` to work with subset of segments and raise error on new segments ([#1103](https://github.com/tinkoff-ai/etna/pull/1103))
Expand Down
3 changes: 2 additions & 1 deletion etna/models/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,8 @@ def forecast(self, ts: "TSDataset", prediction_size: int) -> "TSDataset":
dropna=False,
)
predictions = self.raw_predict(test_dataset)
future_ts = ts.tsdataset_idx_slice(start_idx=self.encoder_length, end_idx=self.encoder_length + prediction_size)
end_idx = len(ts.index)
future_ts = ts.tsdataset_idx_slice(start_idx=end_idx - prediction_size, end_idx=end_idx)
for (segment, feature_nm), value in predictions.items():
# we don't want to change dtype after assignment, but there can happen cast to float32
future_ts.df.loc[:, pd.IndexSlice[segment, feature_nm]] = value[:prediction_size, :].astype(np.float64)
Expand Down
69 changes: 35 additions & 34 deletions tests/test_models/test_inference/test_forecast.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,6 @@ def _test_forecast_in_sample_full_no_target(ts, model, transforms):
def test_forecast_in_sample_full_no_target(self, model, transforms, example_tsds):
self._test_forecast_in_sample_full_no_target(example_tsds, model, transforms)

@to_be_fixed(raises=AssertionError)
# Looks like a problem of current implementation of NNs
@pytest.mark.parametrize(
"model, transforms",
[
(RNNModel(input_size=1, encoder_length=7, decoder_length=7, trainer_params=dict(max_epochs=1)), []),
(
MLPModel(input_size=2, hidden_size=[10], decoder_length=7, trainer_params=dict(max_epochs=1)),
[LagTransform(in_column="target", lags=[5, 6])],
),
],
)
def test_forecast_in_sample_full_no_target_failed_assertion_error(self, model, transforms, example_tsds):
self._test_forecast_in_sample_full_no_target(example_tsds, model, transforms)

@pytest.mark.parametrize(
"model, transforms",
[
Expand All @@ -126,6 +111,21 @@ def test_forecast_in_sample_full_no_target_failed_not_enough_context(self, model
with pytest.raises(ValueError, match="Given context isn't big enough"):
self._test_forecast_in_sample_full_no_target(example_tsds, model, transforms)

@to_be_fixed(raises=AssertionError)
# Looks like a problem of current implementation of NNs
@pytest.mark.parametrize(
"model, transforms",
[
(RNNModel(input_size=1, encoder_length=7, decoder_length=7, trainer_params=dict(max_epochs=1)), []),
(
MLPModel(input_size=2, hidden_size=[10], decoder_length=7, trainer_params=dict(max_epochs=1)),
[LagTransform(in_column="target", lags=[5, 6])],
),
],
)
def test_forecast_in_sample_full_no_target_failed_assertion_error(self, model, transforms, example_tsds):
Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen this pattern while refactoring nn for etna-v2

Why do we duplicate information about assertion raised?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you explain please what exactly do you mean? Naming of the test?

Copy link
Contributor

@martins0n martins0n Feb 14, 2023 •
edited

Choose a reason for hiding this comment

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

@to_be_fixed(raises=AssertionError) and test_forecast_in_sample_full_no_target_failed_assertion_error

  • assertion_error duplication
  • Instead of using test_forecast_in_sample_full_no_target with pytest.mark.xfail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • assertion_error to distinguish different tests that failed in different ways.
  • to_be_fixed is used to clarify that we want to fix that test in the future and I can write a particular error message to catch inside the test.

self._test_forecast_in_sample_full_no_target(example_tsds, model, transforms)

@to_be_fixed(raises=NotImplementedError, match="It is not possible to make in-sample predictions")
@pytest.mark.parametrize(
"model, transforms",
Expand Down Expand Up @@ -181,7 +181,6 @@ class TestForecastInSampleFull:
(HoltModel(), []),
(HoltWintersModel(), []),
(SimpleExpSmoothingModel(), []),
(RNNModel(input_size=1, encoder_length=7, decoder_length=7, trainer_params=dict(max_epochs=1)), []),
],
)
def test_forecast_in_sample_full(self, model, transforms, example_tsds):
Expand All @@ -200,19 +199,6 @@ def test_forecast_in_sample_full_failed_nans_lags(self, model, transforms, examp
with pytest.raises(ValueError, match="Input contains NaN, infinity or a value too large"):
_test_prediction_in_sample_full(example_tsds, model, transforms, method_name="forecast")

@pytest.mark.parametrize(
"model, transforms",
[
(
MLPModel(input_size=2, hidden_size=[10], decoder_length=7, trainer_params=dict(max_epochs=1)),
[LagTransform(in_column="target", lags=[2, 3])],
),
],
)
def test_forecast_in_sample_full_failed_nans_lags_nns(self, model, transforms, example_tsds):
with pytest.raises(AssertionError):
_test_prediction_in_sample_full(example_tsds, model, transforms, method_name="forecast")

@pytest.mark.parametrize(
"model, transforms",
[
Expand All @@ -226,6 +212,21 @@ def test_forecast_in_sample_full_failed_not_enough_context(self, model, transfor
with pytest.raises(ValueError, match="Given context isn't big enough"):
_test_prediction_in_sample_full(example_tsds, model, transforms, method_name="forecast")

@to_be_fixed(raises=AssertionError)
# Looks like a problem of current implementation of NNs
@pytest.mark.parametrize(
"model, transforms",
[
(RNNModel(input_size=1, encoder_length=7, decoder_length=7, trainer_params=dict(max_epochs=1)), []),
(
MLPModel(input_size=2, hidden_size=[10], decoder_length=7, trainer_params=dict(max_epochs=1)),
[LagTransform(in_column="target", lags=[2, 3])],
),
],
)
def test_forecast_in_sample_full_failed_nans_lags_nns(self, model, transforms, example_tsds):
_test_prediction_in_sample_full(example_tsds, model, transforms, method_name="forecast")

@to_be_fixed(raises=NotImplementedError, match="It is not possible to make in-sample predictions")
@pytest.mark.parametrize(
"model, transforms",
Expand Down Expand Up @@ -548,7 +549,7 @@ def _test_forecast_out_sample_suffix(ts, model, transforms, full_prediction_size
# firstly we should forecast prefix to use it as a context
forecast_prefix_ts = deepcopy(forecast_gap_ts)
forecast_prefix_ts.df = forecast_prefix_ts.df.iloc[:-suffix_prediction_size]
model.forecast(forecast_prefix_ts, prediction_size=prediction_size_diff)
forecast_prefix_ts = model.forecast(forecast_prefix_ts, prediction_size=prediction_size_diff)
forecast_gap_ts.df = forecast_gap_ts.df.combine_first(forecast_prefix_ts.df)

# forecast suffix with known context for it
Expand Down Expand Up @@ -583,6 +584,10 @@ def _test_forecast_out_sample_suffix(ts, model, transforms, full_prediction_size
(SeasonalMovingAverageModel(), []),
(NaiveModel(lag=3), []),
(DeadlineMovingAverageModel(window=1), []),
(
MLPModel(input_size=2, hidden_size=[10], decoder_length=7, trainer_params=dict(max_epochs=1)),
[LagTransform(in_column="target", lags=[5, 6])],
),
],
)
def test_forecast_out_sample_suffix(self, model, transforms, example_tsds):
Expand All @@ -594,10 +599,6 @@ def test_forecast_out_sample_suffix(self, model, transforms, example_tsds):
"model, transforms",
[
(RNNModel(input_size=1, encoder_length=7, decoder_length=7, trainer_params=dict(max_epochs=1)), []),
(
MLPModel(input_size=2, hidden_size=[10], decoder_length=7, trainer_params=dict(max_epochs=1)),
[LagTransform(in_column="target", lags=[5, 6])],
),
],
)
def test_forecast_out_sample_suffix_failed_assertion_error(self, model, transforms, example_tsds):
Expand Down