-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix butchering for parsnip's bart()
#263
Conversation
mean(predict(x, newdata = head(mtcars), type = "ppd")), | ||
mean(predict(res, newdata = head(mtcars), type = "ppd")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing the mean because these predictions involve randomness.
mean(predict(x, new_data = head(mtcars))$.pred), | ||
mean(predict(res, new_data = head(mtcars))$.pred), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here:
library(parsnip)
## note that `bart()` has a pretty nasty namespace collision:
tidymodels::tidymodels_prefer()
spec <- bart(mode = "regression")
fitted <- fit(spec, mpg ~ ., mtcars)
predict(fitted, new_data = head(mtcars, n = 3))
#> # A tibble: 3 × 1
#> .pred
#> <dbl>
#> 1 20.8
#> 2 20.6
#> 3 24.4
predict(fitted, new_data = head(mtcars, n = 3))
#> # A tibble: 3 × 1
#> .pred
#> <dbl>
#> 1 20.8
#> 2 20.6
#> 3 24.6
predict(fitted, new_data = head(mtcars, n = 3))
#> # A tibble: 3 × 1
#> .pred
#> <dbl>
#> 1 20.7
#> 2 20.6
#> 3 24.6
Created on 2023-08-16 with reprex v2.0.2
@topepo I just pinged you for a review because Simon is OOO and we need to do a butcher release ahead of the rsample release. Let me know if you aren't able to take a look in the very near future! |
res <- fit(spec, mpg ~ ., mtcars) | ||
x <- butcher(res) | ||
|
||
expect_equal( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add some tests with predict(type = "conf_int")
and predict(type = "pred_int")
too (just in case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @topepo!
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
Closes #262
Turns out we need that
sigma
slot for predicting withtype = "ppd"
as well as for parsnip'bart()
.