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

[ARIMA] Incorrect computation of n #690

Closed
jmoralez opened this issue Nov 9, 2023 · 3 comments · Fixed by #708
Closed

[ARIMA] Incorrect computation of n #690

jmoralez opened this issue Nov 9, 2023 · 3 comments · Fixed by #708
Labels

Comments

@jmoralez
Copy link
Member

jmoralez commented Nov 9, 2023

What happened + What you expected to happen

n here won't have the number of non NA values, but the number minus 1, because the end of the slice isn't included. This was originally reported in #654 since if there's a single sample then n=0 and then we try to take its log.

firstnonmiss = missing_idxs.min()
lastnonmiss = missing_idxs.max()
n = np.sum(~missing[firstnonmiss:lastnonmiss])

Versions / Dependencies

1.6.0

Reproduction script

import numpy as np
from statsforecast.models import AutoARIMA

AutoARIMA().fit(np.array([1.0]))

Issue Severity

None

@jmoralez jmoralez added the bug label Nov 9, 2023
@StatKumar
Copy link
Contributor

@jmoralez Can I work on this issue?

@jmoralez
Copy link
Member Author

Hey @StatKumar. Sure, go ahead! We have a contributing guide here but feel free to ask any questions.

We also have the bug here:

firstnonmiss = np.min(nonmiss_idxs)
lastnonmiss = np.max(nonmiss_idxs)
n = np.sum(~missing[firstnonmiss:lastnonmiss])

and here:
firstnonmiss = nonmissing_idxs.min()
lastnonmiss = nonmissing_idxs.max()
series_len = int(np.sum(~missing[firstnonmiss:lastnonmiss]))

The fix would be adding a +1 to the lastnonmiss, i.e. lastnonmiss = missing_idxs.max() + 1 so that the last index is considered in the slice below.

@StatKumar
Copy link
Contributor

Thank you @jmoralez

StatKumar added a commit to StatKumar/statsforecast that referenced this issue Nov 24, 2023
StatKumar added a commit to StatKumar/statsforecast that referenced this issue Nov 24, 2023
@jmoralez jmoralez linked a pull request Dec 19, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants