Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix config rejection for #1068 #1069

Merged
merged 7 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
8 changes: 6 additions & 2 deletions smac/intensifier/abstract_intensifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, since all the configs are unique and disordered in _rejected_config_ids, I cannot find a reason not to store config_ids as Set()

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
Expand Down
42 changes: 42 additions & 0 deletions tests/test_intensifier/test_abstract_intensifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down