Feat/strided dataset for torch and regression models #2624
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2624 +/- ##
==========================================
- Coverage 94.24% 94.19% -0.05%
==========================================
Files 141 141
Lines 15466 15475 +9
==========================================
+ Hits 14576 14577 +1
- Misses 890 898 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks @madtoinou, this looks very good already 🚀
I left some suggestions, mainly regarding:
- add support to
TorchForecastingModel.fit() - we should extract samples starting from the end of the series in both cases (TFM already does that,
RegressionModelnot yet) - add support to
HorizonBasedDataset
| stride | ||
| The number of time steps between consecutive entries. |
There was a problem hiding this comment.
We could try to make this a bit more informative (and everywhere else where it's documented).
- let's not add another wording like "entries" (we already have samples, osbservations, ...).
- make more clear that we create the training set by extracting samples with
stridefrom the series - does it take from start or end?
- Maybe mention that this should be used with caution regarding predict, historical forecasts, ... that one should apply it only on the same stridden scenario
| @@ -576,6 +585,7 @@ def create_lagged_prediction_data( | |||
| check_inputs: bool = True, | |||
| use_moving_windows: bool = True, | |||
| concatenate: bool = True, | |||
| stride: int = 1, | |||
There was a problem hiding this comment.
should this be supported for prediction?
There was a problem hiding this comment.
I don't think that it should be supported in predict() as it might cause a lot of issues but I need it in create_lagged_prediction_data() for the optimized historical forecast with auto-regression.
There was a problem hiding this comment.
Ahh like roll_size for the torch models @madtoinou? If so then I would use that name here as well :)
There was a problem hiding this comment.
No no, here, I need to be able to generate a strided prediction dataset for historical forecasts in order to predict all the horizons at once. The rolling is going to happen in another function.
CHANGELOG.md
Outdated
| @@ -12,6 +12,7 @@ but cannot always guarantee backwards compatibility. Changes that may **break co | |||
| **Improved** | |||
|
|
|||
| - New model: `StatsForecastAutoTBATS`. This model offers the [AutoTBATS](https://nixtlaverse.nixtla.io/statsforecast/src/core/models.html#autotbats) model from Nixtla's `statsforecasts` library. [#2611](https://github.com/unit8co/darts/pull/2611) by [He Weilin](https://github.com/cnhwl). | |||
| - Added a `stride` argument to the `Dataset` classes (torch-based models) and the fitting methods of the `RegressionModels` to reduce the size of the training set or apply elaborate training approaches. [#2624](https://github.com/unit8co/darts/pull/2529) by [Antoine Madrona](https://github.com/madtoinou) | |||
There was a problem hiding this comment.
I mention somewhere below that it would be good to add stride also to TorchForecastingModel.predict(). Once that is done, we should update the changelog here
Checklist before merging this PR:
Fixes #2621, fixes #1064, fixes #940, fixes #1487.
Summary
strideargument to the torch models datasets and the tabularization methodsOther Information
As discussed offline, using
limit_train_batcheswithmax_samples_per_ts=Noneshould allow to obtain uniformly selected training samples when using torch-based models. Changing the way the samples are retained whenmax_samples_per_ts!=Noneis hence not a priority and will be tackled in a separate PR.