Merge pull request #415 from automl/condition_bugfix

Fix suggestion, needs discussion

Author: thijssnelleman

src/ConfigSpace/configuration_space.py

@@ -508,25 +508,11 @@ def get_active_hyperparameters(
         )
         active_hyperparameters = set()
         for hp_name in self.keys():
-            conditions = self.parent_conditions_of[hp_name]
-
-            active = True
-            for condition in conditions:
-                parent_vector_idx: np.intp | Array[np.intp]
-                if isinstance(condition, Conjunction):
-                    assert condition.parent_vector_ids is not None
-                    parent_vector_idx = condition.parent_vector_ids
-                else:
-                    parent_vector_idx = np.asarray(condition.parent_vector_id)
-
-                if np.isnan(vector[parent_vector_idx]).any():
-                    active = False
-                    break
-
-                if not condition.satisfied_by_vector(vector):
-                    active = False
-                    break
-
+            # NOTE: There could be a performance improvement possible here;
+            # We currently check all conditions, however as a condition with only AND (no OR) can never evaluate to be True
+            # if the vector contains any NaNs. This would have to be a property of the condition, pre-calculated
+            # and could then be used here to reduce the number of checks.
+            active = all(condition.satisfied_by_vector(vector) for condition in self.parent_conditions_of[hp_name])
             if active:
                 active_hyperparameters.add(hp_name)
 

test/test_conditions.py

@@ -30,6 +30,7 @@
 import numpy as np
 import pytest
 
+from ConfigSpace import ConfigurationSpace
 from ConfigSpace.conditions import (
     AndConjunction,
     EqualsCondition,
@@ -378,3 +379,43 @@ def test_get_parents() -> None:
     condition2 = InCondition(counter, _1_0_restarts, ["F", "D", "L", "x", "+"])
     conjunction = AndConjunction(condition, condition2)
     assert [_1_S_countercond, _1_0_restarts] == conjunction.parents
+
+
+def test_active_hyperparameter():
+    cs = ConfigurationSpace(
+        [
+            UniformFloatHyperparameter("age_weight_ratio:log_ratio", -10.0, 3.0, 0.0),
+            CategoricalHyperparameter(
+                "saturation_algorithm",
+                ["discount", "fmb", "inst_gen", "lrs", "otter", "z3"],
+                "lrs",
+            ),
+            CategoricalHyperparameter("inst_gen_with_resolution", ["off", "on"], "off"),
+        ],
+    )
+    cs.add(
+        OrConjunction(
+            NotEqualsCondition(
+                cs["age_weight_ratio:log_ratio"],
+                cs["saturation_algorithm"],
+                "inst_gen",
+            ),
+            EqualsCondition(
+                cs["age_weight_ratio:log_ratio"],
+                cs["inst_gen_with_resolution"],
+                "on",
+            ),
+        ),
+    )
+    cs.add(
+        EqualsCondition(
+            cs["inst_gen_with_resolution"],
+            cs["saturation_algorithm"],
+            "inst_gen",
+        ),
+    )
+
+    # Check that parameter age_weight_ratio:log_ratio is active according to the default configuration
+    # This should be the case, as saturation_algorithm is set to "lrs" (which is NOT "inst_gen") in default.
+    default = cs.get_default_configuration()
+    cs._check_configuration_rigorous(default)

test/test_converters_and_test_searchspaces/test_sample_configuration_spaces.py

@@ -47,9 +47,10 @@
     (this_directory.parent / "test_searchspaces").absolute().resolve()
 )
 pcs_files = list(Path(configuration_space_path).glob("*.pcs"))
+pcs_file_ids = [path.name for path in pcs_files]
 
 
-@pytest.mark.parametrize("pcs_file", pcs_files)
+@pytest.mark.parametrize("pcs_file", pcs_files, ids=pcs_file_ids)
 def test_autosklearn_space(pcs_file: Path):
     with warnings.catch_warnings():
         warnings.simplefilter("ignore")

test/test_searchspaces/vampire.pcs

@@ -0,0 +1,6 @@
+age_weight_ratio:log_ratio real [-10.0, 3.0] [0.0]
+saturation_algorithm categorical {discount, fmb, inst_gen, lrs, otter, z3} [lrs]
+inst_gen_with_resolution categorical {off, on} [off]
+
+age_weight_ratio:log_ratio | saturation_algorithm != inst_gen || inst_gen_with_resolution == on
+inst_gen_with_resolution | saturation_algorithm == inst_gen