Skip to content

Teach AutoARIMAModel to work with out-sample predictions #830

Merged
merged 9 commits into from
Aug 11, 2022

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Aug 2, 2022

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Proposed Changes

Look #790.

Closing issues

Closes #790.

@Mr-Geekman Mr-Geekman self-assigned this Aug 2, 2022
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

🚀 Deployed on https://deploy-preview-830--etna-docs.netlify.app

@github-actions github-actions bot temporarily deployed to pull request August 2, 2022 15:33 Inactive
@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2022

Codecov Report

Merging #830 (1a9a456) into master (2b8a527) will increase coverage by 0.01%.
The diff coverage is 91.95%.

@@            Coverage Diff             @@
##           master     #830      +/-   ##
==========================================
+ Coverage   84.64%   84.65%   +0.01%     
==========================================
  Files         130      130              
  Lines        7455     7409      -46     
==========================================
- Hits         6310     6272      -38     
+ Misses       1145     1137       -8     
Impacted Files Coverage Δ
etna/models/sarimax.py 94.77% <91.25%> (+1.12%) ⬆️
etna/models/autoarima.py 100.00% <100.00%> (+9.58%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@martins0n martins0n self-requested a review August 3, 2022 07:22
@martins0n martins0n marked this pull request as draft August 3, 2022 16:46
@github-actions github-actions bot temporarily deployed to pull request August 9, 2022 10:36 Inactive
@Mr-Geekman Mr-Geekman marked this pull request as ready for review August 9, 2022 12:56
@@ -45,126 +41,11 @@ def __init__(
Training parameters for auto_arima from pmdarima package.
"""
self.kwargs = kwargs
self._model: Optional[ARIMA] = None
self.regressor_columns: List[str] = []
super().__init__()
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be before self.kwargs I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? In class AutoARIMAModel(PerSegmentPredictionIntervalModel) we set self.kwargs before calling super constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

By mistake, no?
It seems logic in parent class could reset self and so it's better to make __init__ call before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And vise-versa, logic in current class can reset the logic in the base class and broke smth.

"Try to encode this columns manually."
)

def get_model(self) -> SARIMAX:
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems we return not Sarimax

@github-actions github-actions bot temporarily deployed to pull request August 11, 2022 06:53 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 11, 2022 07:33 Inactive
@Mr-Geekman Mr-Geekman merged commit 1565b18 into master Aug 11, 2022
@Mr-Geekman Mr-Geekman deleted the issue-790 branch August 11, 2022 07:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Teach AutoARIMAModel to work with out-sample predictions
3 participants