From 06d584f5b1b9614ba2b900b03bf0ad6ef294a6c7 Mon Sep 17 00:00:00 2001
From: PhilippBordne <71140732+PhilippBordne@users.noreply.github.com>
Date: Mon, 9 Oct 2023 11:42:02 +0200
Subject: [PATCH] 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 <difandeng@gmail.com>
Co-authored-by: Difan Deng <33290713+dengdifan@users.noreply.github.com>
---
 CHANGELOG.md                                  |  3 +-
 smac/intensifier/abstract_intensifier.py      |  8 +++-
 .../test_abstract_intensifier.py              | 42 +++++++++++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index aa2d5ae480..5ffa77ab52 100644
--- 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
index b944867273..cb537e9ccb 100644
--- 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
index ce980c49b9..b8dc91a1cb 100644
--- 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