Skip to content

[BUG] Something went wrong with multi-index if backtest (issue #461) #771

Merged
merged 35 commits into from
Jul 13, 2022
Merged

[BUG] Something went wrong with multi-index if backtest (issue #461) #771

Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5e74031
Update base.py
mvakhmenin Jun 22, 2022
cb9685c
Merge pull request #1 from mvakhmenin/patch-1
mvakhmenin Jun 22, 2022
53d3811
Update base.py
mvakhmenin Jun 22, 2022
3633a0c
Update base.py
mvakhmenin Jun 22, 2022
b93ead7
Update base.py
mvakhmenin Jun 22, 2022
e966f63
Update base.py
mvakhmenin Jun 22, 2022
461ffa2
Update base.py
mvakhmenin Jun 22, 2022
95c90cc
Merge branch 'master' into master
mvakhmenin Jun 22, 2022
3dec66b
Update base.py
mvakhmenin Jun 23, 2022
7414e14
Update base.py
mvakhmenin Jun 28, 2022
a5a6677
Update base.py
mvakhmenin Jun 30, 2022
c400082
Update base.py
mvakhmenin Jun 30, 2022
ea3490f
Update base.py
mvakhmenin Jun 30, 2022
8165d76
Merge branch 'tinkoff-ai:master' into master
mvakhmenin Jun 30, 2022
1231261
Update base.py
mvakhmenin Jun 30, 2022
b9fb9e1
Merge branch 'master' into master
martins0n Jul 1, 2022
3eedf50
Update base.py
mvakhmenin Jul 1, 2022
051d856
Update base.py
mvakhmenin Jul 4, 2022
c080fb6
Update base.py
mvakhmenin Jul 4, 2022
4a88a62
Update conftest.py
mvakhmenin Jul 5, 2022
41f100a
Update conftest.py
mvakhmenin Jul 5, 2022
935f648
Update test_pipeline.py
mvakhmenin Jul 5, 2022
b2aaa81
Update test_pipeline.py
mvakhmenin Jul 5, 2022
9f5f01c
Update test_pipeline.py
mvakhmenin Jul 5, 2022
2e576fe
Update test_pipeline.py
mvakhmenin Jul 5, 2022
701a92f
Update test_pipeline.py
mvakhmenin Jul 5, 2022
0381843
Update test_pipeline.py
mvakhmenin Jul 5, 2022
2c8b632
Update test_pipeline.py
mvakhmenin Jul 6, 2022
abc1312
Update CHANGELOG.md
mvakhmenin Jul 6, 2022
6eb031d
make format
mvakhmenin Jul 12, 2022
b505d72
Merge branch 'master' into master
martins0n Jul 12, 2022
b938d01
Update conftest.py
mvakhmenin Jul 13, 2022
3dc128d
Update conftest.py
mvakhmenin Jul 13, 2022
539295c
Merge branch 'master' into master
martins0n Jul 13, 2022
9f2bddd
Merge branch 'master' into master
martins0n Jul 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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 @@ -39,7 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix missing prophet in docker images ([#767](https://github.com/tinkoff-ai/etna/pull/767))
- Add `known_future` parameter to CLI ([#758](https://github.com/tinkoff-ai/etna/pull/758))
- FutureWarning: The frame.append method is deprecated. Use pandas.concat instead ([#764](https://github.com/tinkoff-ai/etna/pull/764))
-
- Correct ordering if multi-index in backtest ([#771](https://github.com/tinkoff-ai/etna/pull/771))
-
-
-
Expand Down
1 change: 1 addition & 0 deletions etna/pipeline/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ def _get_backtest_forecasts(self) -> pd.DataFrame:
forecast = forecast.join(fold_number_df)
forecasts_list.append(forecast)
forecasts = pd.concat(forecasts_list)
forecasts.sort_index(axis=1, inplace=True)
return forecasts
Copy link
Contributor

Choose a reason for hiding this comment

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

We dont need specify ascending order here.

Copy link
Contributor Author

@mvakhmenin mvakhmenin Jun 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@martins0n
Checks failed with the following error. I cannot understand, what should I do
ImportError while loading conftest '/home/runner/work/etna/etna/tests/test_ensembles/conftest.py'. tests/test_ensembles/conftest.py:16: in <module> from etna.models import ProphetModel E ImportError: cannot import name 'ProphetModel' from 'etna.models' (/home/runner/work/etna/etna/etna/models/__init__.py) Error: Process completed with exit code 4.

Copy link
Contributor

@martins0n martins0n Jun 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It's ok, will fix it today or next day

You can make futher changes and test locally, via command poetry run pytest tests -v --cov=etna -m "not long" --cov-report=xml and run linters via make lint

Copy link
Contributor

Choose a reason for hiding this comment

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

@mvakhmenin We've merged updates to master branch. You can rebase on updated master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martins0n
Tests fail without ascending order

Copy link
Contributor

@martins0n martins0n Jul 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@mvakhmenin We don't expect such ordering anyway, it've just happened by chanse.

it's better to change fixtures in conftest to make tests pass

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martins0n
I do not understand, what's the problem with lint

Copy link
Contributor

Choose a reason for hiding this comment

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

You can run linters locally and fix issues with command: make format and make lint

Current error could be fixed with make format I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can find more information about contributing in guide


def _prepare_fold_masks(self, ts: TSDataset, masks: Union[int, List[FoldMask]], mode: str) -> List[FoldMask]:
Expand Down
5 changes: 2 additions & 3 deletions tests/test_pipeline/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,12 @@ def step_ts() -> Tuple[TSDataset, pd.DataFrame, pd.DataFrame]:
target_forecast += [start_value + i * add_value] * horizon
fold_number_forecast += [i] * horizon
forecast_df = pd.DataFrame(
{"target": target_forecast, "fold_number": fold_number_forecast},
{"fold_number": fold_number_forecast, "target": target_forecast},
index=timestamp_forecast,
)
forecast_df.columns = pd.MultiIndex.from_product(
[[segment], ["target", "fold_number"]], names=("segment", "feature")
[[segment], ["fold_number", "target"]], names=("segment", "feature")
)

return ts, metrics_df, forecast_df


Expand Down
7 changes: 7 additions & 0 deletions tests/test_pipeline/test_pipeline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,3 +573,10 @@ def test_backtest_nans_at_beginning_with_mask(ts_name, request):
metrics=[MAE()],
n_folds=[mask],
)


def test_forecast_backtest_correct_ordering(step_ts: TSDataset):
ts, _, expected_forecast_df = step_ts
pipeline = Pipeline(model=NaiveModel(), horizon=5)
_, forecast_df, _ = pipeline.backtest(ts=ts, metrics=[MAE()], n_folds=3)
assert np.all(forecast_df.values == expected_forecast_df.values)