-
Notifications
You must be signed in to change notification settings - Fork 24
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
PLS regression with mixOmics engine - predictions not extracted from predict object #132
Comments
Hi @marioem, thank you for reaching out and for providing a very detailed example. Just from the initial look, it seems as though the prediction wrapper you supplied just needs modified. For RMSE, the prediction wrapper needs to return a single vector of predictions, but it looks like just calling |
the first use of vip in above example uses bare-foot predict which resolves to predict.pls method from mixOmics, with return values documented here. As probably many users of both vip and mixOmics seeing that predict.pls method is available, would put it as is into pred_wrapper, I was wondering if the improvement to |
Hi @marioem, I'm not sure I understand what you mean? The docs in the link specify the output as a list with four components. The proper prediction wrapper extracts the prediction component and works as expected: Best_test_resultsRmse %>%
extract_fit_parsnip() %>%
vip::vip(method = "permute",
num_features = 30,
train = normalized_rec %>% prep() %>% bake(new_data = NULL),
target = "compressive_strength",
metric = "rmse",
nsim = 500,
#######
pred_wrapper = function(object, newdata) { predict(object, newdata)$predict},
#######
geom = "col",
all_permutations = F,
aesthetics = list(color = "grey35"),
include_type = T
) +
ggtitle("Predictor importance - PLS")
Or are you suggesting an improvement to the docs to help prevent the subtle confusion in the proper prediction wrapper to supply? If so, the tidymodels team opened a similar issue here. The docs for I'd be happy to modify the examples, and maybe add a special vignette if you think it'd be helpful for general users? |
update to the documentation would be very helpful. We (users) are spoiled by S3 and we often automatically assume every predict method behaves as predict.lm, and my assumption here was that |
Makes sense to me. I'll put some thought into and make sure the docs are much clearer and will add an example of when this is not the case! Thanks again for posting the issue, I'll close this once the docs are updated accordingly. |
Hi,
vi_permute
fails on PLS perdict object.Created on 2022-10-18 with reprex v2.0.2
This can be worked around by setting
but it would be nice if
vi_
functions were aware of this PLS idiosyncrasy, and worked with it out-of-the-box.Cheers,
Mariusz
The text was updated successfully, but these errors were encountered: