Skip to content
Open
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
25 changes: 15 additions & 10 deletions src/ConfigSpace/_condition_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import numpy as np
from more_itertools import unique_everseen

from ConfigSpace.conditions import Condition, ConditionLike, Conjunction
from ConfigSpace.conditions import Condition, ConditionLike, Conjunction, AndConjunction
from ConfigSpace.exceptions import (
AmbiguousConditionError,
ChildNotFoundError,
Expand Down Expand Up @@ -675,15 +675,17 @@ def add_condition(self, condition: ConditionLike) -> None:

# This stems from the fact all nodes can only have one parent condition,
# as this prevent ambiguity. If a node can have two parents it depends on,
# is it inherently an AND or an OR condition? This
# it is inherently an AND condition: stating a list of conditions or rules implies that they all apply at the same time
# NOTE Option: allow the user to specify whether they want to add the condition as an AND or an OR?
combinator_flag = False
if child.parent_condition is not None and condition != child.parent_condition:
raise AmbiguousConditionError(
"Adding a second parent condition for a for a hyperparameter"
" is ambiguous and therefore forbidden. Use an `OrConjunction`"
" or `AndConjunction` to combine conditions instead."
f"\nAlready inserted: {child.parent_condition}"
f"\nNew one: {condition}",
)
# 1. Merge the conditions with an AND operator
# NOTE: What if one of them already is an AND? Would it be better to merge them into eachother? What if they're both, do we synthesise their union?
condition = AndConjunction(child.parent_condition, condition)
# 2. Clean the existing HP condition from the DAG
child.parent_condition = None
# 3. Add the newly synthesised condition like normal, using the combinator flag to signal this for later checks
combinator_flag = True

parent_names = (
[dlc.parent.name for dlc in condition.dlcs]
Expand All @@ -701,7 +703,10 @@ def add_condition(self, condition: ConditionLike) -> None:
# Ensure there is no existing condition between the parent and the child
_, existing = parent.children.get(child.name, (None, None))
if existing is not None and condition != existing:
raise AmbiguousConditionError(existing, condition)
if combinator_flag: # We are synthesising a new AND condition, so we pop the current condition
parent.children.pop(child.name)
else:
raise AmbiguousConditionError(existing, condition)

# Now we are certain they exist and no existing condition between them,
# update the nodes to point to each other
Expand Down
54 changes: 51 additions & 3 deletions test/test_configuration_space.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
from ConfigSpace.api.types.float import Float
from ConfigSpace.api.types.integer import Integer
from ConfigSpace.exceptions import (
AmbiguousConditionError,
ChildNotFoundError,
CyclicDependancyError,
ForbiddenValueError,
Expand Down Expand Up @@ -341,6 +340,7 @@ def test_add_conjunction():


def test_add_second_condition_wo_conjunction():
# Add a second condition to a hyperparameter
hp1 = CategoricalHyperparameter("input1", [0, 1])
hp2 = CategoricalHyperparameter("input2", [0, 1])
hp3 = Constant("And", "True")
Expand All @@ -352,8 +352,56 @@ def test_add_second_condition_wo_conjunction():
cs.add([hp1, hp2, hp3])
cs.add(cond1)

with pytest.raises(AmbiguousConditionError):
cs.add(cond2)
# Original test, modified to now test that the conditions are correctly updated
cs.add(cond2)
assert len(cs.conditions) == 1
assert cs.conditions[0] == AndConjunction(cond1, cond2)

# Second test: example from https://github.com/automl/ConfigSpace/issues/380#issuecomment-3921050841
cs = ConfigurationSpace()
cs.add(Integer("x", (1, 10), default=1))
cs.add(Integer("y", (1, 10), default=1))
cs.add(Integer("z", (1, 10), default=1))

cond1 = EqualsCondition(cs["z"], cs["x"], 3)
cond2 = EqualsCondition(cs["z"], cs["y"], 3)
cs.add(cond1)
cs.add(cond2)
assert len(cs.conditions) == 1
assert cs.conditions[0] == AndConjunction(cond1, cond2)

# Third test: A larger, more complicated CS with various conditions and multiple updates
cs = ConfigurationSpace()
cs.add(Integer("q", (1, 10), default=1))
cs.add(Integer("w", (1, 10), default=1))
cs.add(Integer("x", (1, 10), default=1))
cs.add(Integer("y", (1, 10), default=1))
cs.add(Integer("z", (1, 10), default=1))

cond1 = AndConjunction(
EqualsCondition(cs["z"], cs["x"], 3),
EqualsCondition(cs["z"], cs["y"], 3),
)
cond2 = OrConjunction(
EqualsCondition(cs["z"], cs["y"], 3),
EqualsCondition(cs["z"], cs["w"], 3),
)
cond3 = OrConjunction(
EqualsCondition(cs["w"], cs["x"], 3),
EqualsCondition(cs["w"], cs["y"], 3),
)
cond4 = AndConjunction(
EqualsCondition(cs["w"], cs["y"], 3),
EqualsCondition(cs["w"], cs["q"], 3),
)
cs.add(cond1)
cs.add(cond2)
cs.add(cond3)
cs.add(cond4)
assert len(cs.conditions) == 2
# NOTE: The order of conditions gets reverted somewhere?
assert cs.conditions[1] == AndConjunction(cond1, cond2)
assert cs.conditions[0] == AndConjunction(cond3, cond4)


def test_add_forbidden_clause():
Expand Down