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

consistently avoid dense matrix conversion for glmnet(x = ...) #1315

Merged
merged 6 commits into from
Mar 21, 2023

Conversation

powerpak
Copy link
Contributor

(This is a more complete version of the fix submitted for #1096.)

Currently, some of the functions for the glmnet model check if the training data is a sparseMatrix, and some don't. The result is that initial operations in train() might succeed, and then later in the workflow, a step will fail (usually with "Cholmod error 'problem too large'" for a sparseMatrix with very large dimensions) because some of the training data is inadvertently converted to a (impossibly large) dense Matrix.

For instance, this bug currently occurs whenever prob() in glmnet.R is called (which happens if trainControl(classProbs = TRUE)), or if tuneLength is used instead of tuneGrid for train(), because tuneLength = ... triggers a call to grid() in glmnet.R which does not check for a sparseMatrix before executing Matrix::as.matrix().

powerpak and others added 6 commits October 22, 2022 18:16
currently, some of the functions for the glmnet model check if the
training data is a sparseMatrix, and some don't. the result is that
initial operations in train() might succeed, and then later in the
workflow, a step will fail (usually with "Cholmod error 'problem too
large'" for a sparseMatrix with very large dimensions) when some of the
training data is inadvertently converted to a dense Matrix. For
instance, this currently would happen when prob() in glmnet.R is called
(if trainControl(classProbs = TRUE)), or if tuneLength is used instead
of tuneGrid in train(), because tuneLength = ... triggers a call to
grid() in glmnet.R which does not do a check for a sparseMatrix.

this is essentially a more complete version of the fix for topepo#1096
@topepo topepo merged commit 31b5faf into topepo:master Mar 21, 2023
@topepo
Copy link
Owner

topepo commented Mar 21, 2023

@powerpak thanks for making this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants