Teach BATS/TBATS to work with in-sample, out-sample predictions correctly
#806
Conversation
|
🚀 Deployed on https://deploy-preview-806--etna-docs.netlify.app |
Codecov Report
@@ Coverage Diff @@
## master #806 +/- ##
==========================================
+ Coverage 84.01% 84.05% +0.04%
==========================================
Files 125 126 +1
Lines 7193 7220 +27
==========================================
+ Hits 6043 6069 +26
- Misses 1150 1151 +1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
etna/models/tbats.py
Outdated
| return self | ||
|
|
||
| def _determine_num_steps_to_forecast(self, last_test_timestamp: pd.Timestamp) -> int: |
There was a problem hiding this comment.
May be we should get this function out of class -- and add it to utils.
I guess something like this logic we have already
| def predict(self, df: pd.DataFrame, prediction_interval: bool, quantiles: Iterable[float]) -> pd.DataFrame: | ||
| if self._fitted_model is None: | ||
| raise ValueError("Model is not fitted! Fit the model before calling predict method!") | ||
|
|
||
| if df["timestamp"].min() <= self._last_train_timestamp: |
There was a problem hiding this comment.
Let's rewrite message smth like: in-sample predictions are not supported by current implementation
etna/models/tbats.py
Outdated
| if df["timestamp"].min() <= self._last_train_timestamp: | ||
| raise NotImplementedError("It is not possible to make in-sample predictions with BATS/TBATS model!") | ||
|
|
||
| steps_to_forecast = self._determine_num_steps_to_forecast(df["timestamp"].max()) |
There was a problem hiding this comment.
May we should raise Exception and stop execution in case if we have mix of in-sample and out-of-sample data?
It looks current behaviour may be error prone
P.S. id we are going to leave it as is. It seems we don't have tests for such cases.
Before submitting (must do checklist)
Proposed Changes
Look #800.
Closing issues
Closes #800.