Implement forecast decomposition for Prophet #1172
Conversation
|
🚀 Deployed on https://deploy-preview-1172--etna-docs.netlify.app |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #1172 +/- ##
===========================================
- Coverage 86.94% 68.37% -18.58%
===========================================
Files 177 177
Lines 10005 10032 +27
===========================================
- Hits 8699 6859 -1840
- Misses 1306 3173 +1867
... and 68 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
etna/models/prophet.py
Outdated
|
|
||
| holiday_names = set(model.train_holiday_names) if model.train_holiday_names is not None else set() | ||
|
|
||
| components_data = {} |
There was a problem hiding this comment.
Can't we do it all at once(not in the cycle)?
There was a problem hiding this comment.
We can try to do here vecorization, but main reason of this solution is to persist consistency with similar method from prophet. I don't think we will gain much speed up vectorizing this cycle. Also, we can't throw out the cycle fully as we need to filter individual holidays and aggregated components from columns in component cols.
There was a problem hiding this comment.
First of all, we can filter out the unnecessary components before the cycle, the second thing is that Prophet team does not plan to change the model, so we don't need to maintain code consistency if it can be vectorized
etna/models/prophet.py
Outdated
| @@ -135,10 +134,7 @@ def predict(self, df: pd.DataFrame, prediction_interval: bool, quantiles: Sequen | |||
| DataFrame with predictions | |||
| """ | |||
| df = df.reset_index() | |||
There was a problem hiding this comment.
Why didn't you put this line inside the _prepare_prophet_df ? You can add this line in fit too
etna/models/prophet.py
Outdated
|
|
||
| holiday_names = set(model.train_holiday_names) if model.train_holiday_names is not None else set() | ||
|
|
||
| components_data = {} |
There was a problem hiding this comment.
First of all, we can filter out the unnecessary components before the cycle, the second thing is that Prophet team does not plan to change the model, so we don't need to maintain code consistency if it can be vectorized
Before submitting (must do checklist)
Proposed Changes
Closing issues
closes #1161