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

Evalcast: set get_predictions default to parallel=FALSE #623

Merged
merged 2 commits into from
May 9, 2023

Conversation

dshemetov
Copy link
Collaborator

@dshemetov dshemetov commented May 8, 2023

Not sure why this was set to TRUE, but using parallelism for forecasting is quite tricky due to the many levels where it can be implemented. We've had too many times in production forecasting where eager parallelism settings at a lower level caused memory or other performance issues. Setting the default to FALSE is the safer option.

Some things to look out for with this change:

  • evalcast dashboard possibly becoming single-threaded and slow
  • same for forecasting pipeline code

The CI is going to fail due to Python tests getting fixed in #620.

@dshemetov dshemetov changed the base branch from main to evalcast May 8, 2023 23:04
@dshemetov dshemetov changed the base branch from evalcast to dplyr-summarize May 9, 2023 08:14
Copy link
Collaborator

@brookslogan brookslogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Could alter roxygen @param doc to indicate the default, but not essential.

@krivard krivard removed their request for review May 9, 2023 19:48
@dshemetov dshemetov merged commit 7aab778 into dplyr-summarize May 9, 2023
@dshemetov dshemetov deleted the ds/get_predictions branch May 9, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants