-
Notifications
You must be signed in to change notification settings - Fork 892
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
base: master
Are you sure you want to change the base?
Imp/Add new model StatsForecastAutoTBATS #2611
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2611 +/- ##
==========================================
- Coverage 94.15% 94.09% -0.07%
==========================================
Files 139 140 +1
Lines 14992 15028 +36
==========================================
+ Hits 14116 14140 +24
- Misses 876 888 +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. 🤝 |
Checklist before merging this PR:
Fixes #2609.
Summary
Add new model StatsForecastAutoTBATS from nixtla.