Merge branch 'main' into fix/autofselector-ro-bindings

Author: be-marc

DESCRIPTION

@@ -24,7 +24,7 @@ URL: https://mlr3fselect.mlr-org.com,
     https://github.com/mlr-org/mlr3fselect
 BugReports: https://github.com/mlr-org/mlr3fselect/issues
 Depends:
-    mlr3 (>= 1.0.1),
+    mlr3 (>= 1.7.1.9000),
     R (>= 3.1.0)
 Imports:
     bbotk (>= 1.9.0),
@@ -46,10 +46,11 @@ Suggests:
     processx,
     redux,
     rpart,
-    rush (>= 1.0.0),
+    rush (>= 1.2.1.9000),
     testthat (>= 3.0.0)
 Remotes:
-    mlr-org/mlr3
+    mlr-org/mlr3,
+    mlr-org/rush
 Config/testthat/edition: 3
 Config/testthat/parallel: false
 Encoding: UTF-8

NEWS.md

@@ -1,6 +1,13 @@
 # mlr3fselect (development version)
 
 * fix: `ArchiveAsyncFSelect` pushed results with the removed `rush::Rush$push_results()` method.
+* fix: `fs("rfecv")` left the resampling of the objective set to an insample resampling, so subsequent evaluations on the same instance silently resampled in-sample (#187).
+* fix: The `mlr3fselect.backup` callback deleted the backup of the previous batch before it wrote the new one, so a crash in between lost the complete run. The benchmark result is now written to a temporary file and renamed afterwards (#188).
+* BREAKING CHANGE: The `mlr3fselect.backup` callback requires the `path` argument now. Previously it wrote a `bmr.rds` file into the working directory of the user (#188).
+* fix: `as.data.table()` on an `EnsembleFSResult` accepts the documented `benchmark_result` argument now to omit the task, learner and resampling columns (#190).
+* fix: The `$print()` methods of `ArchiveBatchFSelect`, `ArchiveAsyncFSelect`, `ArchiveAsyncFSelectFrozen`, `AutoFSelector` and `FSelector` errored with `unused argument` when arguments such as `digits` were passed (#190).
+* fix: `fs("rfecv")` had the same label as `fs("rfe")`, so both were indistinguishable in `as.data.table(mlr_fselectors)`. Its manual page also instructed to construct it with `fs("rfe")` (#191).
+* fix: `AutoFSelector$train()` did not check the row ids of an instantiated inner resampling for cross-validation and reported a wrong set number for holdout (#197).
 * fix: The `mlr3fselect.svm_rfe` callback accepted support vector machines without a `type` or `kernel` setting, although only `type = "C-classification"` and `kernel = "linear"` are supported. The callback now also errors on multi-class tasks for which the importance scores are not defined (#173).
 * fix: The asynchronous feature selection ignored the `always_included` column role. Columns with this role were excluded from the models instead of being added to every feature subset (#175).
 * fix: The `mlr3fselect.one_se_rule` callback errored on archives with a single evaluation or with missing scores, and wrote the `n_features` column as a list column instead of an integer column (#174).

R/ArchiveAsyncFSelect.R

@@ -47,9 +47,6 @@ ArchiveAsyncFSelect = R6Class(
   public = list(
     #' @description
     #' Creates a new instance of this [R6][R6::R6Class] class.
-    #'
-    #' @param check_values (`logical(1)`)\cr
-    #'   If `TRUE` (default), feature subsets are check for validity.
     initialize = function(
       search_space,
       codomain,
@@ -123,7 +120,7 @@ ArchiveAsyncFSelect = R6Class(
     #' Printer.
     #'
     #' @param ... (ignored).
-    print = function() {
+    print = function(...) {
       cat_cli(cli_h1("{format(self)} with {.val {self$n_evals}} evaluations"))
       print(
         as.data.table(

R/ArchiveAsyncFSelectFrozen.R

@@ -83,7 +83,7 @@ ArchiveAsyncFSelectFrozen = R6Class(
     #' Printer.
     #'
     #' @param ... (ignored).
-    print = function() {
+    print = function(...) {
       cat_cli(cli_h1("{.cls {class(self)[1L]}} with {.val {self$n_evals}} evaluations"))
       print(
         as.data.table(

R/ArchiveBatchFSelect.R

@@ -155,7 +155,7 @@ ArchiveBatchFSelect = R6Class(
     #' Printer.
     #'
     #' @param ... (ignored).
-    print = function() {
+    print = function(...) {
       cat_cli(cli_h1("{.cls {class(self)[1]}}"))
       print(self$data[, setdiff(names(self$data), "uhash"), with = FALSE], digits = 2)
     },

R/AutoFSelector.R

@@ -188,7 +188,7 @@ AutoFSelector = R6Class(
     #' @return Named `numeric()`.
     importance = function() {
       if ("importance" %nin% self$instance_args$learner$properties) {
-        stopf("Learner ''%s' cannot calculate important scores.", self$instance_args$learner$id)
+        stopf("Learner '%s' cannot calculate importance scores.", self$instance_args$learner$id)
       }
       if (is.null(self$model$learner$model)) {
         self$instance_args$learner$importance()
@@ -204,7 +204,7 @@ AutoFSelector = R6Class(
     #' @return `character()`.
     selected_features = function() {
       if ("selected_features" %nin% self$instance_args$learner$properties) {
-        stopf("Learner ''%s' cannot select features.", self$instance_args$learner$id)
+        stopf("Learner '%s' cannot select features.", self$instance_args$learner$id)
       }
       if (is.null(self$model$learner$model)) {
         self$instance_args$learner$selected_features()
@@ -243,9 +243,11 @@ AutoFSelector = R6Class(
       }
     },
 
+    #' @description
     #' Printer.
+    #'
     #' @param ... (ignored).
-    print = function() {
+    print = function(...) {
       msg_h = if (is.null(self$label) || is.na(self$label)) "" else paste0(": ", self$label)
       model = if (is.null(self$model)) "-" else class(self$model)[1L]
 
@@ -359,29 +361,22 @@ AutoFSelector = R6Class(
       ia$task = task$clone()
 
       # check if task contains all row ids required for instantiated resampling
+      # `$train_set()` and `$test_set()` are used because the layout of `$instance` differs between resamplings
       if (ia$resampling$is_instantiated) {
-        imap(ia$resampling$instance$train, function(x, i) {
-          if (!test_subset(x, task$row_ids)) {
-            stopf(
-              "Train set %i of inner resampling '%s' contains row ids not present in task '%s': {%s}",
-              i,
-              ia$resampling$id,
-              task$id,
-              paste(setdiff(x, task$row_ids), collapse = ", ")
-            )
-          }
-        })
-
-        imap(ia$resampling$instance$test, function(x, i) {
-          if (!test_subset(x, task$row_ids)) {
-            stopf(
-              "Test set %i of inner resampling '%s' contains row ids not present in task '%s': {%s}",
-              i,
-              ia$resampling$id,
-              task$id,
-              paste(setdiff(x, task$row_ids), collapse = ", ")
-            )
-          }
+        walk(seq_len(ia$resampling$iters), function(i) {
+          sets = list(Train = ia$resampling$train_set(i), Test = ia$resampling$test_set(i))
+          imap(sets, function(row_ids, set_type) {
+            if (!test_subset(row_ids, task$row_ids)) {
+              stopf(
+                "%s set %i of inner resampling '%s' contains row ids not present in task '%s': {%s}",
+                set_type,
+                i,
+                ia$resampling$id,
+                task$id,
+                paste(setdiff(row_ids, task$row_ids), collapse = ", ")
+              )
+            }
+          })
         })
       }
 

R/EnsembleFSResult.R

@@ -214,7 +214,7 @@ EnsembleFSResult = R6Class(
 
       # check if `inner_measure` is an `mlr3::Measure`
       if (which == "inner" && is.null(private$.inner_measure)) {
-        stop("No inner_measure was defined during initialization")
+        error_input("No `inner_measure` was defined during initialization.")
       }
 
       private$.active_measure = which
@@ -277,7 +277,7 @@ EnsembleFSResult = R6Class(
         }
 
         # Combine results from both objects
-        private$.result = data.table::rbindlist(list(private$.result, result2), fill = FALSE)
+        private$.result = rbindlist(list(private$.result, result2), fill = FALSE)
 
         # Merge benchmark results if available in both objects
         has_bmr = !is.null(self$benchmark_result)
@@ -523,7 +523,7 @@ EnsembleFSResult = R6Class(
         pf = pf[n_features > 0]
 
         # Fit the linear model
-        form = mlr3misc::formulate(lhs = measure_id, rhs = "n_features_inv")
+        form = formulate(lhs = measure_id, rhs = "n_features_inv")
         model = stats::lm(formula = form, data = pf)
 
         # Predict values using the model to create a smooth curve
@@ -671,8 +671,9 @@ EnsembleFSResult = R6Class(
 )
 
 #' @export
-as.data.table.EnsembleFSResult = function(x, ...) {
-  x$result
+as.data.table.EnsembleFSResult = function(x, ..., benchmark_result = TRUE) {
+  assert_flag(benchmark_result)
+  if (benchmark_result) x$result else copy(get_private(x)$.result)
 }
 
 #' @export

R/FSelector.R

@@ -73,8 +73,10 @@ FSelector = R6Class(
     #' @description
     #' Print method.
     #'
+    #' @param ... (ignored).
+    #'
     #' @return (`character()`).
-    print = function() {
+    print = function(...) {
       msg_h = if (is.na(self$label)) "" else paste0(": ", self$label)
       cat_cli({
         cli_h1("{.cls {class(self)[1L]}}{msg_h}")
@@ -96,7 +98,7 @@ FSelector = R6Class(
     #' Set of control parameters.
     param_set = function(rhs) {
       if (!missing(rhs) && !identical(rhs, private$.param_set)) {
-        stop("$param_set is read-only.")
+        error_input("`$param_set` is read-only.")
       }
       private$.param_set
     },
@@ -106,7 +108,7 @@ FSelector = R6Class(
     #' Must be a subset of [`mlr_reflections$fselect_properties`][mlr3::mlr_reflections].
     properties = function(rhs) {
       if (!missing(rhs) && !identical(rhs, private$.properties)) {
-        stop("$properties is read-only.")
+        error_input("`$properties` is read-only.")
       }
       private$.properties
     },
@@ -116,7 +118,7 @@ FSelector = R6Class(
     #' Note that these packages will be loaded via [requireNamespace()], and are not attached.
     packages = function(rhs) {
       if (!missing(rhs) && !identical(rhs, private$.packages)) {
-        stop("$packages is read-only.")
+        error_input("`$packages` is read-only.")
       }
       private$.packages
     },
@@ -126,7 +128,7 @@ FSelector = R6Class(
     #' Can be used in tables, plot and text output instead of the ID.
     label = function(rhs) {
       if (!missing(rhs) && !identical(rhs, private$.param_set)) {
-        stop("$label is read-only.")
+        error_input("`$label` is read-only.")
       }
       private$.label
     },
@@ -136,14 +138,14 @@ FSelector = R6Class(
     #' The referenced help package can be opened via method `$help()`.
     man = function(rhs) {
       if (!missing(rhs) && !identical(rhs, private$.man)) {
-        stop("$man is read-only.")
+        error_input("`$man` is read-only.")
       }
       private$.man
     }
   ),
 
   private = list(
-    .optimize = function(inst) stop("abstract"),
+    .optimize = function(inst) error_bbotk("`.optimize()` is abstract and must be implemented by the subclass."),
 
     .assign_result = function(inst) {
       assert_fselect_instance(inst)

R/FSelectorAsyncFromOptimizerAsync.R

@@ -53,7 +53,7 @@ FSelectorAsyncFromOptimizerAsync = R6Class(
     #' Set of control parameters.
     param_set = function(rhs) {
       if (!missing(rhs) && !identical(rhs, private$.optimizer$param_set)) {
-        stop("$param_set is read-only.")
+        error_input("`$param_set` is read-only.")
       }
       private$.optimizer$param_set
     }

R/FSelectorBatchGeneticSearch.R

@@ -56,7 +56,7 @@ FSelectorBatchGeneticSearch = R6Class(
       }
       n = inst$objective$domain$length
 
-      mlr3misc::invoke(genalg::rbga.bin, size = n, evalFunc = inst$objective_function, .args = pars)
+      invoke(genalg::rbga.bin, size = n, evalFunc = inst$objective_function, .args = pars)
     }
   )
 )