[WIP] ARIMA model (CSS fitting) #174
Conversation
|
The two notebooks in the example folder are not necessary. I would keep the ARIMA documentation as internal reference. What is the purpose of the other one? |
| from gtime.forecasting import ARIMAForecaster | ||
|
|
||
|
|
||
| class TestNaiveModel(SimplePipelineTest): |
There was a problem hiding this comment.
These tests are not enough to test well ARIMA
There was a problem hiding this comment.
I still need to review the part in stat_tools.
I see also better what you did. The tests in forecasting package are less important since the core is in stat_tools. If you manage you can add a couple of tests without using hypothesis, so it's easier.
Regarding stat_tools I'm not sure if it's a good idea to create a new package in this way, but I think we can postpone the decision since it will depend on the future directions of the library.
| model = ARMAMLEModel((self.n_ar, self.n_ma), self.method) | ||
| model.fit(np_x) | ||
| self._set_params(model, np_x) | ||
| super().fit(X, y) |
There was a problem hiding this comment.
It is not clear to me why you call two times a fit function. Can you explain it to me?
gtime/forecasting/arima.py
Outdated
| X: np.array, difference of ``self.order[1]`` order of X | ||
|
|
||
| """ | ||
| n = len(X) |
There was a problem hiding this comment.
No need of this variable since is it used only one time
There was a problem hiding this comment.
I still need to store it because after that length of X changes as I add previous values for AR
gtime/forecasting/arima.py
Outdated
| return eps | ||
|
|
||
|
|
||
| class ARIMAForecaster(SimpleForecaster): |
There was a problem hiding this comment.
I refactored the simple models calling them naive.
Also, I'm not sure about inheriting from the parent class of the naive forecasters. Either we create a parent class for the arima forecasters or we create a parent class for all the forecasters
deatinor
force-pushed
the
master
branch
3 times, most recently
from
03fc53f
to
3b881b2
Compare
Reference Issues/PRs
What does this implement/fix? Explain your changes.
ARIMA model fitted with conditional sum of squares approximation.
Any other comments?