Fix config rejection for #1068 (#1069)

* racing: make sure config only deleted from rejected configs if among incumbents upon update * test case for incumbent rejection * update code comments * added missing newline ahead of new test case * update change log --------- Co-authored-by: dengdifan <[email protected]> Co-authored-by: Difan Deng <[email protected]>
automl · Oct 9, 2023 · 06d584f · 06d584f
1 parent d58d5b1
commit 06d584f
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,8 +1,9 @@
 # 2.0.3
 
 ## Bugfixes
-- Add OrdinalHyperparameter for random forest imputer (#1065).
 - Fix path for dask scheduler file (#1055).
+- Add OrdinalHyperparameter for random forest imputer (#1065).
+- Configurations that fail to become incumbents will be added to the rejected lists (#1069).
 
 # 2.0.2
 

diff --git a/smac/intensifier/abstract_intensifier.py b/smac/intensifier/abstract_intensifier.py
@@ -571,8 +571,12 @@ def update_incumbents(self, config: Configuration) -> None:
 
         if len(previous_incumbents) == len(new_incumbents):
             if previous_incumbents == new_incumbents:
-                # No changes in the incumbents
-                self._remove_rejected_config(config_id)
+                # No changes in the incumbents, we need this clause because we can't use set difference then
+                if config_id in new_incumbent_ids:
+                    self._remove_rejected_config(config_id)
+                else:
+                    # config worse than incumbents and thus rejected
+                    self._add_rejected_config(config_id)
                 return
             else:
                 # In this case, we have to determine which config replaced which incumbent and reject it

diff --git a/tests/test_intensifier/test_abstract_intensifier.py b/tests/test_intensifier/test_abstract_intensifier.py
@@ -109,6 +109,48 @@ def test_incumbent_selection_multi_objective(make_scenario, configspace_small, m
     assert intensifier.get_incumbents() == [config]
 
 
+def test_config_rejection_single_objective(configspace_small, make_scenario):
+    """ Tests whether configs are rejected properly if they are worse than the incumbent. """
+    scenario = make_scenario(configspace_small, use_instances=False)
+    runhistory = RunHistory()
+    intensifier = Intensifier(scenario=scenario)
+    intensifier.runhistory = runhistory
+
+    configs = configspace_small.sample_configuration(3)
+
+    runhistory.add(config=configs[0],
+                   cost=5,
+                   time=0.0,
+                   seed=0,
+                   status=StatusType.SUCCESS,
+                   force_update=True)
+    intensifier.update_incumbents(configs[0])
+
+    assert intensifier._rejected_config_ids == []
+
+    # add config that yielded better results, updating incumbent and sending prior incumbent to rejected
+    runhistory.add(config=configs[1],
+                   cost=1,
+                   time=0.0,
+                   seed=0,
+                   status=StatusType.SUCCESS,
+                   force_update=True)
+    intensifier.update_incumbents(config=configs[1])
+
+    assert intensifier._rejected_config_ids == [1]
+
+    # add config that is no better should thus go to rejected
+    runhistory.add(config=configs[2],
+                   cost=1,
+                   time=0.0,
+                   seed=0,
+                   status=StatusType.SUCCESS,
+                   force_update=True)
+    intensifier.update_incumbents(config=configs[2])
+
+    assert intensifier._rejected_config_ids == [1, 3]
+
+
 def test_incumbent_differences(make_scenario, configspace_small):
     pass