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

Fix butchering for parsnip's bart() #263

Merged
merged 4 commits into from
Aug 22, 2023
Merged

Fix butchering for parsnip's bart() #263

merged 4 commits into from
Aug 22, 2023

Conversation

juliasilge
Copy link
Member

Closes #262

Turns out we need that sigma slot for predicting with type = "ppd" as well as for parsnip' bart().

Comment on lines +36 to +37
mean(predict(x, newdata = head(mtcars), type = "ppd")),
mean(predict(res, newdata = head(mtcars), type = "ppd")),
Copy link
Member Author

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.

Comment on lines +49 to +50
mean(predict(x, new_data = head(mtcars))$.pred),
mean(predict(res, new_data = head(mtcars))$.pred),
Copy link
Member Author

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

@juliasilge juliasilge requested review from simonpcouch and topepo and removed request for simonpcouch August 16, 2023 22:51
@juliasilge
Copy link
Member Author

@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!

@hfrick hfrick mentioned this pull request Aug 21, 2023
24 tasks
res <- fit(spec, mpg ~ ., mtcars)
x <- butcher(res)

expect_equal(
Copy link
Member

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).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @topepo!

@juliasilge juliasilge merged commit 872624d into main Aug 22, 2023
@juliasilge juliasilge deleted the fix-bart-predict branch August 22, 2023 18:59
@github-actions
Copy link

github-actions bot commented Sep 6, 2023

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.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with predicting after butchering bart
2 participants