Read a randomForest stump without getTree - #367
Merged
Conversation
A tree with a single root node and no split made tidypredict_fit() and parse_model() abort with "argument of length 0", a base-R error coming out of randomForest itself. randomForest::getTree() assembles the node table with cbind() and then subsets it to the tree's node count without drop = FALSE. For a stump that count is 1, so the matrix collapses to a plain vector, nrow() returns NULL, and the 1:nrow(tree) on the next line fails before the tree is ever returned. Nothing in tidypredict was reached. Both call sites now go through rf_get_tree(), which delegates to getTree() for a tree with more than one node and assembles the same six columns itself for a stump. The rest of the parser and the nested formula builder already handled a terminal root node. A stump appears whenever the outcome is constant within a bootstrap sample, which a constant outcome or a zero-variance predictor makes routine, and predict() scores such a forest fine. Closes #362
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #362
A tree with a single root node and no split made
tidypredict_fit()andparse_model()abort with "argument of length 0".Root cause
The error comes from
randomForestitself, before any tidypredict code runs.randomForest::getTree()assembles the node table withcbind()and subsets it to the tree's node count withoutdrop = FALSE. For a stump that count is 1, so the matrix collapses to a plain vector,nrow()returnsNULL, and the1:nrow(tree)on the next line fails.Fix
Both call sites (
rf_tree_info_full()for theparse_model()path andbuild_nested_rf_tree()for the directtidypredict_fit()path) now go throughrf_get_tree(), which delegates togetTree()for a tree with more than one node and assembles the same six columns itself for a stump. The parser and the nested formula builder already handled a terminal root node, so nothing downstream changed.Tests
Two tests in
tests/testthat/test-model-rf.R, both comparing againstrandomForest::predict()on the direct and the parsed path, and both asserting the fit actually contains a stump. Verified they fail on the unpatched parser.