Open marioem opened 1 year ago
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 predict()
on this type of object returns a list? If so, maybe you're looking for something closer to predict(object, newdata)$predict
(or whatever the name of the actual vector of predictions is called). Is this the case here?
Hi @brandongreenwell-8451 ,
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 predwrapper, I was wondering if the improvement to `vifunctions would be possible to make this type of return value transparent to the user. Examples in documentation of
vi_permute` use plain predict and that primed me to think that this function would handle every predict method provided by the packages implementing models.
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 vi_permute()
explain that for regression problems, the prediction wrapper should return a numeric (and atomic) vector of prediction, which does mean that the user will have to be familiar with what the generic predict()
will return for their model objects.
I'd be happy to modify the examples, and maybe add a special vignette if you think it'd be helpful for general users?
Hi @brandongreenwell-8451 ,
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 pred_wrapper = predict
is all that is required for vi_permute
to work with mixOmics methods as well.
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