search

mlr-org / mlr3filters

Filter-based feature selection for mlr3
https://mlr3filters.mlr-org.com
GNU Lesser General Public License v3.0
20 stars 8 forks source link

`FilterVariableImportance()` fails for `classif.ranger` #20

Closed pat-s closed 5 years ago

pat-s commented 5 years ago 
library(mlr3)
library(mlr3learners)
library(mlr3featsel)
lrn = mlr_learners$get("classif.ranger", param_vals = list(importance = "impurity"))
task = mlr_tasks$get("iris")
filter = FilterVariableImportance$new(learner = lrn)
filter$calculate(task)
#> INFO  [16:30:10.241] Training learner 'classif.ranger' on task 'iris' ...
#> Error: No importance stored

Created on 2019-06-04 by the reprex package (v0.3.0)

The reason is that

https://github.com/mlr-org/mlr3featsel/blob/fc91dbc3540c2e05dae859990fbade854a6d7b06/R/FilterVariableImportance.R#L40

overwrites the param_set slot of learner which has the importance = "impurity" information here that is needed during training. Do we need this line? After the clone (in l.39) everything should be present in the learner object?

Also we need a test for this. The current one does not catch this bug since classif.rpart, which is used in the test, does not rely on a external param_vals to be set.

pat-s commented 5 years ago

Seems to work now again. Maybe I was not on the latest version? Strange.