-
Notifications
You must be signed in to change notification settings - Fork 917
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
Feat/statsforecasts #893
Feat/statsforecasts #893
Conversation
Codecov Report
@@ Coverage Diff @@
## master #893 +/- ##
==========================================
- Coverage 91.57% 91.55% -0.02%
==========================================
Files 71 73 +2
Lines 7165 7246 +81
==========================================
+ Hits 6561 6634 +73
- Misses 604 612 +8
Continue to review full report at Codecov.
|
|
||
mu = forecast_df["mean"].values | ||
if num_samples > 1: | ||
std = 2 * (forecast_df["hi_68%"].values - mu) |
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.
Why is there a factor 2? Isn't hi_68 - mu equal to 1 std?
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.
Yes, same question
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.
Unless I missed something, With a 68% forecast interval, it means that "hi - mu" represents 34% of the mass. (and "mu - low" represents the other 34%). So I multiply by 2 to get back one std (2x 34%). Makes sense?
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.
OK my bad that was right ;) Now fixed.
from darts.timeseries import TimeSeries | ||
|
||
|
||
class Croston(ForecastingModel): |
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.
Apparently TSB and croston_sba are improved version of Croston (https://www.lancaster.ac.uk/pg/waller/pdfs/Intermittent_Demand_Forecasting.pdf), might be worth proposing each of the 3 options in the same function as they already exist in statsforecast?
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.
Good idea, done.
I've implemented a couple of models coming from Statsforecasts. The most interesting is probably their implementation of AutoARIMA, which uses Numba (and jit compilation) and can in some cases be much faster than pmdarima (after the first call to
fit()
). I would propose that we add it first as a separate model, and if everything goes well and it proves superior to the current AutoARIMA version, we could even replace it entirely at some point (it is not quite the same model, though).