-
Notifications
You must be signed in to change notification settings - Fork 894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Imp/Add new model StatsForecastAutoTBATS #2611
Imp/Add new model StatsForecastAutoTBATS #2611
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2611 +/- ##
==========================================
- Coverage 94.25% 94.19% -0.07%
==========================================
Files 140 141 +1
Lines 15427 15463 +36
==========================================
+ Hits 14541 14565 +24
- Misses 886 898 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR @cnhwl, it looks very good and is almost ready to be merged 🚀
Before merging could you address the points from below?
- add entry to
CHANGELOG.md
- add model to the
README.md
- add model to
darts/userguide/covariates.md
in this table - run probabilistic tests by adding the model to
darts/tests/models/forecasting/test_probabilistic_models.py
Co-authored-by: Dennis Bader <[email protected]>
Thank you very much for your code review and guidance! @dennisbader. I'll finish the points above soon. 🤝 |
Hi! @dennisbader I'm trying to add the models to |
Hi @cnhwl, our capacity is currently a bit limited and we still need some time to get to. Sorry for that, but we will get there eventually! |
I see, that is true :) Actually, in my conformal prediction PR #2552, I introduce the @random_method that can be used by any model to control the randomness (see here). This will allow you to control the sampling. Once we merge the PR (I think by end of next week), we can add this to our StatsForecastModels as well. Is it okay for you to wait until then? Otherwise, the PR looks great and is ready to be merged :) Thanks a lot for the great work @cnhwl 🚀 |
Thanks again for your help! @dennisbader Of course! I am more than willing to wait for your PR #2552, so please let me know when that happens. 🤝 |
Hi @dennisbader, I've resolved the code conflict and am ready to merge anytime now! 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great now, thanks @cnhwl 🚀
I can add probabilistic support with randomness handling later for all statsforecast models.
Checklist before merging this PR:
Fixes #2609.
Summary
Add new model StatsForecastAutoTBATS from nixtla.