fix: AcqFunction domain construction respects Surrogate cols_x now (#129)

sumny · web-flow · commit e3383c777208 · 2023-08-23T14:47:41.000+02:00
* docs: infill --&gt; acquisition function

* fix: AcqFunction domain construction respects Surrogate cols_x now
diff --git a/R/AcqFunction.R b/R/AcqFunction.R
@@ -60,10 +60,9 @@ AcqFunction = R6Class("AcqFunction",
         }
         private$.surrogate = surrogate
         private$.archive = assert_r6(surrogate$archive, classes = "Archive")
-        codomain = generate_acq_codomain(surrogate$archive$codomain, id = id, direction = direction)
-        self$surrogate_max_to_min = surrogate_mult_max_to_min(surrogate$archive$codomain, cols_y = surrogate$cols_y)
-        domain = surrogate$archive$search_space$clone(deep = TRUE)
-        domain$trafo = NULL
+        codomain = generate_acq_codomain(surrogate, id = id, direction = direction)
+        self$surrogate_max_to_min = surrogate_mult_max_to_min(surrogate)
+        domain = generate_acq_domain(surrogate)
       }
       super$initialize(id = id, domain = domain, codomain = codomain, constants = constants)
     },
@@ -160,7 +159,7 @@ AcqFunction = R6Class("AcqFunction",
     },
 
     #' @field fun (`function`)\cr
-    #'   Pointing to the private acquisition function to be implemented by subclasses.
+    #'   Points to the private acquisition function to be implemented by subclasses.
     fun = function(lhs) {
       if (!missing(lhs) && !identical(lhs, private$.fun)) stop("$fun is read-only.")
       private$.fun
@@ -178,10 +177,9 @@ AcqFunction = R6Class("AcqFunction",
         }
         private$.surrogate = rhs
         private$.archive = assert_r6(rhs$archive, classes = "Archive")
-        codomain = generate_acq_codomain(rhs$archive$codomain, id = self$id, direction = self$direction)
-        self$surrogate_max_to_min = surrogate_mult_max_to_min(rhs$archive$codomain, cols_y = rhs$cols_y)
-        domain = rhs$archive$search_space$clone(deep = TRUE)
-        domain$trafo = NULL
+        codomain = generate_acq_codomain(rhs, id = self$id, direction = self$direction)
+        self$surrogate_max_to_min = surrogate_mult_max_to_min(rhs)
+        domain = generate_acq_domain(rhs)
         self$codomain = Codomain$new(codomain$params)  # lazy initialization requires this
         self$domain = domain
       }
diff --git a/R/SurrogateLearner.R b/R/SurrogateLearner.R
@@ -23,7 +23,7 @@
 #' }
 #' \item{`catch_errors`}{`logical(1)`\cr
 #'   Should errors during updating the surrogate be caught and propagated to the `loop_function` which can then handle
-#'   the failed infill optimization (as a result of the failed surrogate) appropriately by, e.g., proposing a randomly sampled point for evaluation?
+#'   the failed acquisition function optimization (as a result of the failed surrogate) appropriately by, e.g., proposing a randomly sampled point for evaluation?
 #'   Default is `TRUE`.
 #' }
 #' }
diff --git a/R/SurrogateLearnerCollection.R b/R/SurrogateLearnerCollection.R
@@ -25,7 +25,7 @@
 #' }
 #' \item{`catch_errors`}{`logical(1)`\cr
 #'   Should errors during updating the surrogate be caught and propagated to the `loop_function` which can then handle
-#'   the failed infill optimization (as a result of the failed surrogate) appropriately by, e.g., proposing a randomly sampled point for evaluation?
+#'   the failed acquisition function optimization (as a result of the failed surrogate) appropriately by, e.g., proposing a randomly sampled point for evaluation?
 #'   Default is `TRUE`.
 #' }
 #' }
diff --git a/R/helper.R b/R/helper.R
@@ -1,18 +1,27 @@
-generate_acq_codomain = function(codomain, id, direction = "same") {
-  assert_choice(direction, c("same", "minimize", "maximize"))
+generate_acq_codomain = function(surrogate, id, direction = "same") {
+  assert_r6(surrogate$archive, classes = "Archive")
+  assert_string(id)
+  assert_choice(direction, choices = c("same", "minimize", "maximize"))
   if (direction == "same") {
-    if (codomain$length > 1L) {
+    if (surrogate$archive$codomain$length > 1L) {
       stop("Not supported yet.")  # FIXME: But should be?
     }
-    tags = codomain$params[[1L]]$tags
+    tags = surrogate$archive$codomain$params[[1L]]$tags
     tags = tags[tags %in% c("minimize", "maximize")]  # only filter out the relevant one
   } else {
     tags = direction
   }
   codomain = ParamSet$new(list(
     ParamDbl$new(id, tags = tags)
   ))
-  return(codomain)
+  codomain
+}
+
+generate_acq_domain = function(surrogate) {
+  assert_r6(surrogate$archive, classes = "Archive")
+  domain = surrogate$archive$search_space$clone(deep = TRUE)$subset(surrogate$cols_x)
+  domain$trafo = NULL
+  domain
 }
 
 archive_xy = function(archive) {
@@ -46,10 +55,16 @@ calculate_parego_weights = function(s, k) {
   matrix(unlist(fun(s, k)), ncol = k, byrow = TRUE) / s
 }
 
-surrogate_mult_max_to_min = function(codomain, cols_y) {
+surrogate_mult_max_to_min = function(surrogate) {
+  codomain = surrogate$archive$codomain
+  cols_y = surrogate$cols_y
   mult = map_int(cols_y, function(col_y) {
-    mult = if (col_y %in% codomain$ids()) {
-      if(has_element(codomain$tags[[col_y]], "maximize")) -1L else 1L
+    mult = if (col_y %in% surrogate$archive$codomain$ids()) {
+      if (has_element(surrogate$archive$codomain$tags[[col_y]], "maximize")) {
+        -1L
+      } else {
+        1L
+      }
     } else {
       1L
     }
@@ -58,7 +73,7 @@ surrogate_mult_max_to_min = function(codomain, cols_y) {
 }
 
 mult_max_to_min = function(codomain) {
-  ifelse(map_lgl(codomain$tags, has_element, "minimize"), 1, -1)
+  ifelse(map_lgl(codomain$tags, has_element, "minimize"), yes = 1L, no = -1L)
 }
 
 # used in AcqOptimizer
@@ -86,7 +101,9 @@ catn = function(..., file = "") {
 }
 
 set_collapse = function(x) {
-  if (length(x) == 0L) return("{}")
+  if (length(x) == 0L) {
+    return("{}")
+  }
   sprintf("{'%s'}", paste0(unique(x), collapse = "','"))
 }
 
@@ -95,14 +112,14 @@ check_attributes = function(x, attribute_names) {
   if (any(attribute_names %nin% names(attributes(x)))) {
     return(sprintf("Attributes must include '%s' but is '%s'", set_collapse(attribute_names), set_collapse(names(attributes(x)))))
   }
-  return(TRUE)
+  TRUE
 }
 
 check_instance_attribute = function(x) {
   if (length(intersect(c("single-crit", "multi-crit"), attr(x, "instance"))) == 0L) {
     return(sprintf("'instance' attribute must be a subset of '%s' but is '%s'", set_collapse(c("single-crit", "multi-crit")), set_collapse(attr(x, "instance"))))
   }
-  return(TRUE)
+  TRUE
 }
 
 check_learner_surrogate = function(learner) {
@@ -118,7 +135,9 @@ check_learner_surrogate = function(learner) {
 }
 
 assert_loop_function = function(x, .var.name = vname(x)) {
-  if (is.null(x)) return(x)
+  if (is.null(x)) {
+    return(x)
+  }
   # NOTE: this is buggy in checkmate; assert should always return x invisible not TRUE as is the case here
   assert(check_class(x, classes = "loop_function"),
          check_function(x, args = c("instance", "surrogate", "acq_function", "acq_optimizer")),
diff --git a/man/AcqFunction.Rd b/man/AcqFunction.Rd
diff --git a/man/SurrogateLearner.Rd b/man/SurrogateLearner.Rd
diff --git a/man/SurrogateLearnerCollection.Rd b/man/SurrogateLearnerCollection.Rd
diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R
@@ -49,29 +49,32 @@ FUN_1D_2_MIXED = function(xs) {
 }
 OBJ_1D_2_MIXED = ObjectiveRFun$new(fun = FUN_1D_2_MIXED, domain = PS_1D_MIXED, codomain = FUN_1D_2_CODOMAIN, properties = "multi-crit")
 
-# Simple 2D Function
-PS_2D_domain = ParamSet$new(list(
+# Simple 2D Functions
+PS_2D = ParamSet$new(list(
   ParamDbl$new("x1", lower = -1, upper = 1),
-  ParamDbl$new("x2", lower = -1, upper = 1),
-  ParamUty$new("foo")  # the domain of the function should not matter
+  ParamDbl$new("x2", lower = -1, upper = 1)
 ))
-PS_2D = ParamSet$new(list(
+PS_2D_trafo = ParamSet$new(list(
   ParamDbl$new("x1", lower = -1, upper = 1),
   ParamDbl$new("x2", lower = -1, upper = 1)
 ))
+PS_2D_trafo$trafo = function(x, param_set) {
+  x$x2 = x$x2 ^ 2
+  x
+}
 FUN_2D = function(xs) {
   y = sum(as.numeric(xs)^2)
   list(y = y)
 }
 FUN_2D_CODOMAIN = ParamSet$new(list(ParamDbl$new("y", tags = c("minimize", "random_tag"))))
-OBJ_2D = ObjectiveRFun$new(fun = FUN_2D, domain = PS_2D_domain, properties = "single-crit")
+OBJ_2D = ObjectiveRFun$new(fun = FUN_2D, domain = PS_2D, properties = "single-crit")
 
 # Simple 2D Function with noise
 FUN_2D_NOISY = function(xs) {
   y = sum(as.numeric(xs)^2) + rnorm(1, sd = 0.5)
   list(y = y)
 }
-OBJ_2D_NOISY = ObjectiveRFun$new(fun = FUN_2D_NOISY, domain = PS_2D_domain, properties = c("single-crit", "noisy"))
+OBJ_2D_NOISY = ObjectiveRFun$new(fun = FUN_2D_NOISY, domain = PS_2D, properties = c("single-crit", "noisy"))
 
 # Instance helper
 MAKE_INST = function(objective = OBJ_2D, search_space = PS_2D, terminator = trm("evals", n_evals = 10L)) {
diff --git a/tests/testthat/test_AcqFunction.R b/tests/testthat/test_AcqFunction.R
@@ -37,3 +37,36 @@ test_that("AcqFunction packages works", {
   expect_equal(acqf$packages, "mlr3mbo")
 })
 
+test_that("AcqFunction generate_acq_codomain works", {
+  inst = MAKE_INST(OBJ_2D, search_space = PS_2D)
+  surrogate = SurrogateLearner$new(REGR_FEATURELESS, archive = inst$archive)
+  codomain = generate_acq_codomain(surrogate, "acqf")
+  expect_r6(codomain, "ParamSet")
+  expect_equal(codomain$tags[["acqf"]], "minimize")
+})
+
+test_that("AcqFunction generate_acq_domain works", {
+  inst = MAKE_INST(OBJ_2D, search_space = PS_2D)
+  surrogate = SurrogateLearner$new(REGR_FEATURELESS, archive = inst$archive)
+  domain = generate_acq_domain(surrogate)
+  expect_equal(domain, OBJ_2D$domain)
+  expect_equal(domain, inst$search_space)
+
+  inst = MAKE_INST(OBJ_2D, search_space = PS_2D_trafo)
+  expect_true(inst$search_space$has_trafo)
+  surrogate = SurrogateLearner$new(REGR_FEATURELESS, archive = inst$archive)
+  domain = generate_acq_domain(surrogate)
+  expect_equal(domain, OBJ_2D$domain)
+  expect_false(domain$has_trafo)
+
+  surrogate$cols_x = "x2"
+  domain = generate_acq_domain(surrogate)
+  surrogate$cols_x = "x2"
+  domain = generate_acq_domain(surrogate)
+  expect_equal(domain, OBJ_2D$domain$clone(deep = TRUE)$subset("x2"))
+  expect_false(domain$has_trafo)
+
+  surrogate = SurrogateLearner$new(REGR_FEATURELESS)
+  expect_error(generate_acq_domain(surrogate), "Must be an R6 class, not 'NULL'")
+})
+
