Bug fix for Issue 2639 #2646
Bug fix for Issue 2639 #2646
Conversation
When using the RF min depth filter with a method other than "md" (i.e. "vh" or "vh.vimp"), an error is generated at line 298: Error in -im[, 1L]: invalid argument to unary operator. This happens because im = NULL. From code im = (result of RF var.select)$md.obj$order From randomForestSRC documentation: md.obj - Minimal depth object. NULL unless method = "md". Therefore the object md.obj cannot be used for methods "vh" and "vh.vimp". The solution is to use object varselect instead, which is a matrix containing the tree depth and relative frequency for each variable. Note also that object topvars contains the names of the top variables.
|
Sorry, I have never done this before and I don't really know what to do next. Could I have some guidance please? |
| @@ -276,8 +276,8 @@ rf.min.depth = makeFilter( | |||
| supported.features = c("numerics", "factors", "ordered"), | |||
| fun = function(task, nselect, method = "md", ...) { | |||
| im = randomForestSRC::var.select(getTaskFormula(task), getTaskData(task), | |||
| method = method, verbose = FALSE, ...)$md.obj$order | |||
| setNames(-im[, 1L], rownames(im)) | |||
| method = method, verbose = FALSE, ...)$varselect | |||
There was a problem hiding this comment.
Please don't change the indentation.
|
Thanks for the PR, looks good apart from some minor issues I raised in the review. You can find our contributor guidelines in the wiki at https://github.com/mlr-org/mlr3/wiki/pr-guidelines For bug fixes, we ask that you write a unit test that checks whether the bug is fixed in addition to the fix itself. This is to make sure that we don't accidentally reintroduce the same bug in future changes. Could you do that please? Thanks! |
Was there an upstream change in randomForestSRC? Reading your initial post is not easy, please take a few more minutes next time to format it nicely. Thanks. |
|
It's a bit more complicated. First, @annette987 is right that Hence, so far we only allowed for However, to make all methods available with a robust behavior, we cannot simply use the I added support for I'll close here since #2649 should solve the issue. |
When using the RF min depth filter with a method other than "md" (i.e.
"vh" or "vh.vimp"), an error is generated at line 298:
Error in -im[, 1L]: invalid argument to unary operator.
This happens because im = NULL.
From code: im = (result of RF var.select)$md.obj$order
From randomForestSRC documentation: md.obj - Minimal depth object. NULL
unless method = "md".
Therefore the object md.obj cannot be used for methods "vh" and "vh.vimp".
The solution is to use object varselect instead, which is a matrix
containing the tree depth and relative frequency for each variable. Note
also that object topvars contains the names of the top variables.
P.S. This is my first PR and I don;t have access to the coding guidelines or other docs. Could I be added please?