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

Allow data validation to be skipped to improve posterior_predict performance with newdata #1521

Merged
merged 4 commits into from
Jul 4, 2023

Conversation

lunafazio
Copy link
Contributor

@lunafazio lunafazio commented Jul 4, 2023

When posterior_predict is called with newdata, validate_newdata will be called and this represents a non-trivial amount of the total execution time. Advanced users may want to skip this step when they know their data already has the proper structure.

As there are different kinds of validation functions in the package, the proposed argument name is skip_validate_data.

library(brms)
fit <- brm(time | cens(censored) ~ age + sex + (1 + age || patient),
           data = kidney, family = "exponential", init = "0",
           backend = "cmdstanr")

microbenchmark::microbenchmark(
  posterior_predict(fit, newdata = fit$data),
  posterior_predict(fit, newdata = fit$data, skip_validate_data = TRUE)
)
# Unit: milliseconds
#                                          expr      min       lq     mean   median       uq      max neval
#                             posterior_predict 228.1443 252.5301 274.1673 266.1105 277.0043 570.7657   100
#  posterior_predict(skip_validate_data = TRUE) 130.8804 149.5136 158.3647 155.1540 163.1465 414.9703   100

The change is made for both current_data and current_data2 and should work without issue when passed via ... from standata and pp_check calls.

Two calls of current_data in get_refmodel had ... added so the argument can also be used there.

There is one current_data2 call left without passing ... as it's inside validate_newdata so it'd be pointless there.

@paul-buerkner
Copy link
Owner

Thanks! Is there a reason to not call it skip_validate? I liked that name in our discussion today.

@lunafazio
Copy link
Contributor Author

Named and style tweaked as requested.

Also future reminder that the argument may be reused or renamed depending on how we want it to interact with other types of non-data validation steps.

@paul-buerkner
Copy link
Owner

great. Merging now.

@paul-buerkner paul-buerkner merged commit 1f211db into paul-buerkner:master Jul 4, 2023
4 of 6 checks passed
@lunafazio lunafazio deleted the skip-validate branch July 4, 2023 12:53
paul-buerkner added a commit that referenced this pull request Jul 4, 2023
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