Skip to content
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

[ENH] expose fitted params for statsforecast forecasters via get_fitted_params #5720

Open
fkiraly opened this issue Jan 9, 2024 · 13 comments · May be fixed by #6349
Open

[ENH] expose fitted params for statsforecast forecasters via get_fitted_params #5720

fkiraly opened this issue Jan 9, 2024 · 13 comments · May be fixed by #6349
Labels
enhancement Adding new functionality feature request New feature or request module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting

Comments

@fkiraly
Copy link
Collaborator

fkiraly commented Jan 9, 2024

We should ensure that statsforecast estimators expose their fitted parameters through get_fitted_params. For that, parameters need to be written in _fit, to attributes ending in underscore.

The changes should be done in the _GeneralisedStatsforecastAdapter, but useful "good first issue" contributions would already be to check where the parameters are stored in statsforecast, e.g., where does Auto-ARIMA store its fitted parameters?


Asked by @holderhe in #931:

how about the StatsForecastAutoARIMA ? how can i retrieve parameter for evaluation like AIC, etc. ?

I think this is not currently supported, but could be done easily by writing these to attributes ending in underscore aftere the fit.

FYI @yarnabrina due to authorship of the statsforecast adapter.

@fkiraly fkiraly added module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting enhancement Adding new functionality labels Jan 9, 2024
yarnabrina added a commit that referenced this issue Feb 17, 2024
…ors (#5920)

## Per estimator lower bound

I checked the imports in `_get_statsforecast_class` for all direct
estimators, and first tag in StatsForecast repository where that class
is present. Accordingly, updated the `python_dependencies` tag per
concrete estimators as follows:

1. `AutoARIMA`: `v1.0.0`
2. `AutoCES`: `v1.1.0`
3. `AutoTheta`: `v1.3.0`
4. `AutoTheta`: `v1.3.2`
5. `ARCH` and `GARCH`: `v1.5.0` (FYI @eyjo)
6. `MSTL`: `v1.2.0` (FYI @luca-miniati)

## Missing credits

1. @arnaujc91 has contributed significantly to
`_GeneralisedStatsForecastAdapter` by adding support for argument
differences across versions, added in the `authors`.
2. Both @arnaujc91 and @luca-miniati (author of
`StatsForecastBackAdapter`) were missing from `__author__`, added there.
3. Added `StatsForecastBackAdapter` in `__all__`.
4. Author/maintainer information for @eyjo was missing for
`StatsForecastARCH`, added there.

## Enhancements

1. `v1.7.1` adds a new `phi` parameter to `AutoETS`, added support for
the same.
2. `pydocstyle` was failing due to missing docstrings, added one liner
summaries (unlikely to be user facing methods).

## MSTL changes

1. `StatsForecastMSTL` was initialising the attributes after call to
`super().__init__`, which was causing `AttributeError` in tests that
check for presence of properties. This PR moves those initialisations
prior to that call. FYI @luca-miniati.
2. Disabled probabilistic capability of `StatsForecastMSTL`, as it was
not working as reported by @EliasKng in #5720. FYI @luca-miniati.

## Update of lower/upper bounds

1. No existing direct estimator support `statsforecast<1.0.0` with their
current imports, not even `AutoARIMA`, updated lower bound from `0.5.2`
to `1.0.0`. Previous adapter by @FedericoGarza was importing from a
different location. `v0.5.2` was released in March 2022, and `v1.0.0`
was released in August 2022, so I think chances of affecting users is
quite low.
3. Recently added `AutoTBATS` needs `v1.7.2`, so updated upper bound
from `1.7.0` to `1.8.0`.
@yarnabrina
Copy link
Collaborator

Duplicate/Superset of #6342 ?

@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 27, 2024

yes, superset of #6342

@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 27, 2024

@holderhe, @jiayuanteng, @yarnabrina, does PR #6349 make the parameters accessible that you want?

@jiayuanteng
Copy link

Yes!

@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 27, 2024

Well, that's good to hear.

I thought is was something more involved, but it turns out the forwarding was just missing.

@fkiraly fkiraly removed the good first issue Good for newcomers label Apr 27, 2024
@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 27, 2024

(removing "good first issue" since resolved)

@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 27, 2024

Unfortunately we missed the Apr feature release by a day, so I hope it's ok if it will be in one of the May releases.

@jiayuanteng
Copy link

No worries. May is fine. I appreciate that you are working on it😄 Just to confirm - will the May release cover the issue #931 i.e., expose AIC, BIC, estimated coefficients in auto aroma function?

@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 27, 2024

if #6349 solves it and does not cause problems, then yes, it will be in one of the May releases.

As you have confirmed that the PR solves it for you, I suppose yes?

What would also be great, if you have 10, 20min, if you could contribute some code for your use case as a test, in a new PR? Specifically for the auto-arima? So we know that we don't break it again.

I suggest forecasting.basetests.test_adapter_statsforecast (new file)

@yarnabrina
Copy link
Collaborator

yarnabrina commented Apr 28, 2024

I am very confused, how does this PR solve the fitted parameters for you @jiayuanteng ? I thought you wanted to get which (p,d,q) combination won (plus corresponding AIC/BIC) as part of AutoARIMA, is that not the case? This PR is not going to expose those if I am not wrong. There is no logic that I can find that creates p_, d_, q_ etc. attributes.

@jiayuanteng
Copy link

No worries. May is fine. I appreciate that you are working on it😄 Just to confirm - will the May release cover the issue #931 i.e., expose AIC, BIC, estimated coefficients in auto aroma function?

@yarnabrina got it. I hope I didn't misunderstand. yeah I wanted to get estimated coefficients and AIC etc. I asked above and @fkiraly confirmed that could be done. Separately, I found a thread on stackoverflow that shows a way to get AIC etc from statsforecast auto ARIMA, the model specification is confusing though (see the comment section to the top answer)

https://stackoverflow.com/questions/75290492/how-to-get-model-specification-paramters-for-models-estimated-with-nixtlas-stat

@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 28, 2024

@jiayuanteng, does the PR solve it for you or not, now?

@yarnabrina, I thought that, if the statsforecast estimator already stored this as underscored attributes, then get_fitted_params would not see it, because there is no attribute forwarding. The PR solves it, in a case where statsforecast does this, by forwarding the attribs.

@yarnabrina
Copy link
Collaborator

@jiayuanteng Thanks for your link. We do not use StatsForecast class directly, and we use the algorithm instances directly. They also seem to have this model_ property, but its format seems to vary a lot. I created a issue on Nixtla repo, hopefully we shall know from that.

Nixtla/statsforecast#829

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding new functionality feature request New feature or request module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting
Projects
None yet
3 participants