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

Bug in predict.train #347

Closed
jknowles opened this issue Jan 6, 2016 · 4 comments
Closed

Bug in predict.train #347

jknowles opened this issue Jan 6, 2016 · 4 comments
Labels
bug

Comments

@jknowles
Copy link
Contributor

@jknowles jknowles commented Jan 6, 2016

I think this is causing our issue over on zachmayer/caretEnsemble#180

Here's a simple minimal reproducible example:

library(caret)
myGBM <- train(x=iris[, 1:2], y = iris[, 5], 
tuneLength = 1, method = "gbm", 
trControl = trainControl(method = "cv", number =2, 
savePredictions = "final", classProbs = FALSE), verbose = FALSE)

predict(myGBM) # results in error
predict(myGBM, newdata = NULL) # results in error

Here is the error message:

Error in object$var.levels[[i]] : subscript out of bounds
Called from: predict.gbm(modelFit, newdata, type = "response", 
n.trees = modelFit$tuneValue$n.trees)

This behavior is inconsistent, as the following works:

myRF <- train(x=iris[, 1:2], y = iris[, 5], 
tuneLength = 1, method = "rf", 
trControl = trainControl(method = "cv", number =2, 
savePredictions = "final", classProbs = FALSE), verbose = FALSE)

predict(myRF) # results in no error
predict(myRF, newdata = NULL) # results in no error

@zachmayer and @topepo -- any advice about investigating whether this is just limited to the predict.gbm implementation? How might we investigate it further?

@topepo
Copy link
Owner

@topepo topepo commented Jan 12, 2016

In the last version, I changed predict.train to avoid using extractPrediction and go straight to predictionFunction. In doing so I didn't account for the use of predict(obj) with no newdata.

It's generally not a good idea to do that but I will fix it since it was working before I "fixed" the other issue =]

@topepo topepo added the bug label Jan 12, 2016
@jknowles
Copy link
Contributor Author

@jknowles jknowles commented Jan 13, 2016

Thanks Max. If it's not a good idea to do predict(modelobj) without newdata, perhaps in the case where our users do not specify newdata in caretEnsemble we should modify the predict call to get the fitted values from the train objects instead of doing using the predict function.

What do you think @zachmayer ?

@zachmayer
Copy link
Collaborator

@zachmayer zachmayer commented Jan 29, 2016

@jknowles How about if it's not a good idea to call predict.train without newdata, we disable calling predict.caretEnsemble without new data?

topepo added a commit that referenced this issue Mar 21, 2016
@topepo
Copy link
Owner

@topepo topepo commented Mar 21, 2016

Do these changes fix the issue? The examples that you've shown don't show errors now.

@topepo topepo closed this Apr 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.