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

Prophet panics when preprocessing data with NaNs #209

Closed
sd2k opened this issue Dec 19, 2024 · 1 comment · Fixed by #219
Closed

Prophet panics when preprocessing data with NaNs #209

sd2k opened this issue Dec 19, 2024 · 1 comment · Fixed by #219
Assignees
Labels
bug Something isn't working

Comments

@sd2k
Copy link
Collaborator

sd2k commented Dec 19, 2024

When the training data contains NaNs, Prophet's preprocess method panics:

panicked at /augurs/crates/augurs-prophet/src/prophet/prep.rs:318:40:
index out of bounds: the len is 297 but the index is 297

This is because we filter ds and y to remove NaNs, but don't adjust n accordingly (or any of the other seasonality/regressor fields, while we're at it).

We should move this filtering into a TrainingData::filter_nans() method. Ideally it would even consume self and return a new TrainingData, which would mean we could avoid having a mutable binding to data in preprocess and be sure we're not inadvertently mutating it anywhere else.

@sd2k sd2k added the bug Something isn't working label Dec 19, 2024
sd2k added a commit that referenced this issue Dec 23, 2024
@sd2k sd2k assigned sd2k and unassigned gerboland Dec 23, 2024
@sd2k
Copy link
Collaborator Author

sd2k commented Dec 23, 2024

@gerboland I've pushed up a fix for this in #219 (I wanted to get it out in a 0.8.0 release).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants