From 84899b723abbdd2f62e2fb6f0dd31e473c9cae23 Mon Sep 17 00:00:00 2001 From: Bogdan Budescu Date: Tue, 19 Nov 2024 19:16:41 +0200 Subject: [PATCH 1/6] avoid evaluating forbidden relations on HPOs that were disabled/not sampled because of conditions that were not met --- src/ConfigSpace/forbidden.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/ConfigSpace/forbidden.py b/src/ConfigSpace/forbidden.py index 4b6ba0d4..0af71ef1 100644 --- a/src/ConfigSpace/forbidden.py +++ b/src/ConfigSpace/forbidden.py @@ -620,7 +620,10 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: def is_forbidden_vector_array(self, arr: Array[f64]) -> Mask: left = arr[self.vector_ids[0]] right = arr[self.vector_ids[1]] - return self.left.to_value(left) < self.right.to_value(right) + mask = ~(np.isnan(left) | np.isnan(right)) + out = np.zeros_like(mask) + out[mask] = self.left.to_value(left[mask]) < self.right.to_value(right[mask]) + return out class ForbiddenEqualsRelation(ForbiddenRelation): @@ -686,7 +689,10 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: def is_forbidden_vector_array(self, arr: Array[f64]) -> Mask: left = arr[self.vector_ids[0]] right = arr[self.vector_ids[1]] - return self.left.to_value(left) == self.right.to_value(right) # type: ignore + mask = ~(np.isnan(left) | np.isnan(right)) + out = np.zeros_like(mask) + out[mask] = self.left.to_value(left[mask]) == self.right.to_value(right[mask]) + return out # type: ignore class ForbiddenGreaterThanRelation(ForbiddenRelation): @@ -751,7 +757,10 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: def is_forbidden_vector_array(self, arr: Array[f64]) -> Mask: left = arr[self.vector_ids[0]] right = arr[self.vector_ids[1]] - return self.left.to_value(left) > self.right.to_value(right) + mask = ~(np.isnan(left) | np.isnan(right)) + out = np.zeros_like(mask) + out[mask] = self.left.to_value(left[mask]) > self.right.to_value(right[mask]) + return out ForbiddenLike = Union[ From 166e8aa5f7def17a1e7e528979d7c361a4bbbe8d Mon Sep 17 00:00:00 2001 From: Bogdan Budescu Date: Wed, 20 Nov 2024 08:49:54 +0200 Subject: [PATCH 2/6] - add nan checks for single vals, not just arrays, before evaluating forbidden relations on conditioned parameters - regression test for crash --- src/ConfigSpace/forbidden.py | 6 ++++++ test/test_forbidden.py | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/ConfigSpace/forbidden.py b/src/ConfigSpace/forbidden.py index 0af71ef1..a3fcbacc 100644 --- a/src/ConfigSpace/forbidden.py +++ b/src/ConfigSpace/forbidden.py @@ -614,6 +614,8 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: # Relation is always evaluated against actual value and not vector rep left: f64 = vector[self.vector_ids[0]] # type: ignore right: f64 = vector[self.vector_ids[1]] # type: ignore + if np.isnan(left) or np.isnan(right): + return False return self.left.to_value(left) < self.right.to_value(right) # type: ignore @override @@ -683,6 +685,8 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: # Relation is always evaluated against actual value and not vector rep left = vector[self.vector_ids[0]] right = vector[self.vector_ids[1]] + if np.isnan(left) or np.isnan(right): + return False return self.left.to_value(left) == self.right.to_value(right) # type: ignore @override @@ -751,6 +755,8 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: # Relation is always evaluated against actual value and not vector rep left: f64 = vector[self.vector_ids[0]] # type: ignore right: f64 = vector[self.vector_ids[1]] # type: ignore + if np.isnan(left) or np.isnan(right): + return False return self.left.to_value(left) > self.right.to_value(right) # type: ignore @override diff --git a/test/test_forbidden.py b/test/test_forbidden.py index f297a6a8..0c9c56c5 100644 --- a/test/test_forbidden.py +++ b/test/test_forbidden.py @@ -291,3 +291,18 @@ def test_relation(): assert forb.is_forbidden_value( {"water_temperature": "hot", "water_temperature2": "cold"}, ) + + +def test_relation_conditioned(): + from ConfigSpace import EqualsCondition, ConfigurationSpace + a = OrdinalHyperparameter('a', [2, 5, 10]) + enable_a = CategoricalHyperparameter('enable_a', [False, True]) + cond_a = EqualsCondition(a, enable_a, True) + + b = OrdinalHyperparameter('b', [5, 10, 15]) + forbid_a_b = ForbiddenEqualsRelation(a, b) + # forbid_a = ForbiddenEqualsClause(enable_a, True) + # forbid_a_b = ForbiddenAndConjunction(forbid_a, forbid_a_b) + + cs = ConfigurationSpace() + cs.add([a, enable_a, cond_a, b, forbid_a_b]) From e5c525c207e7ae071354e46518815b95110f7d24 Mon Sep 17 00:00:00 2001 From: Bogdan Budescu Date: Wed, 20 Nov 2024 09:10:43 +0200 Subject: [PATCH 3/6] - aesthetic refactor: rename for improved legibility - finalize regression test --- src/ConfigSpace/forbidden.py | 18 +++++++++--------- test/test_forbidden.py | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ConfigSpace/forbidden.py b/src/ConfigSpace/forbidden.py index a3fcbacc..0f1d5ad3 100644 --- a/src/ConfigSpace/forbidden.py +++ b/src/ConfigSpace/forbidden.py @@ -622,9 +622,9 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: def is_forbidden_vector_array(self, arr: Array[f64]) -> Mask: left = arr[self.vector_ids[0]] right = arr[self.vector_ids[1]] - mask = ~(np.isnan(left) | np.isnan(right)) - out = np.zeros_like(mask) - out[mask] = self.left.to_value(left[mask]) < self.right.to_value(right[mask]) + valid = ~(np.isnan(left) | np.isnan(right)) + out = np.zeros_like(valid) + out[valid] = self.left.to_value(left[valid]) < self.right.to_value(right[valid]) return out @@ -693,9 +693,9 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: def is_forbidden_vector_array(self, arr: Array[f64]) -> Mask: left = arr[self.vector_ids[0]] right = arr[self.vector_ids[1]] - mask = ~(np.isnan(left) | np.isnan(right)) - out = np.zeros_like(mask) - out[mask] = self.left.to_value(left[mask]) == self.right.to_value(right[mask]) + valid = ~(np.isnan(left) | np.isnan(right)) + out = np.zeros_like(valid) + out[valid] = self.left.to_value(left[valid]) == self.right.to_value(right[valid]) return out # type: ignore @@ -763,9 +763,9 @@ def is_forbidden_vector(self, vector: Array[f64]) -> bool: def is_forbidden_vector_array(self, arr: Array[f64]) -> Mask: left = arr[self.vector_ids[0]] right = arr[self.vector_ids[1]] - mask = ~(np.isnan(left) | np.isnan(right)) - out = np.zeros_like(mask) - out[mask] = self.left.to_value(left[mask]) > self.right.to_value(right[mask]) + valid = ~(np.isnan(left) | np.isnan(right)) + out = np.zeros_like(valid) + out[valid] = self.left.to_value(left[valid]) > self.right.to_value(right[valid]) return out diff --git a/test/test_forbidden.py b/test/test_forbidden.py index 0c9c56c5..6891a2e1 100644 --- a/test/test_forbidden.py +++ b/test/test_forbidden.py @@ -295,14 +295,14 @@ def test_relation(): def test_relation_conditioned(): from ConfigSpace import EqualsCondition, ConfigurationSpace + a = OrdinalHyperparameter('a', [2, 5, 10]) - enable_a = CategoricalHyperparameter('enable_a', [False, True]) + enable_a = CategoricalHyperparameter('enable_a', [False, True], weights=[99999, 1]) cond_a = EqualsCondition(a, enable_a, True) b = OrdinalHyperparameter('b', [5, 10, 15]) forbid_a_b = ForbiddenEqualsRelation(a, b) - # forbid_a = ForbiddenEqualsClause(enable_a, True) - # forbid_a_b = ForbiddenAndConjunction(forbid_a, forbid_a_b) cs = ConfigurationSpace() cs.add([a, enable_a, cond_a, b, forbid_a_b]) + cs.sample_configuration(100) From 1673f3afdf3b4b0c46cdb9908115c5f9516d09ef Mon Sep 17 00:00:00 2001 From: Bogdan Budescu Date: Wed, 20 Nov 2024 09:17:05 +0200 Subject: [PATCH 4/6] test all forbidden relations, not just equals --- test/test_forbidden.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/test_forbidden.py b/test/test_forbidden.py index 6891a2e1..5d287e93 100644 --- a/test/test_forbidden.py +++ b/test/test_forbidden.py @@ -301,8 +301,9 @@ def test_relation_conditioned(): cond_a = EqualsCondition(a, enable_a, True) b = OrdinalHyperparameter('b', [5, 10, 15]) - forbid_a_b = ForbiddenEqualsRelation(a, b) + for forbid in ForbiddenEqualsRelation, ForbiddenGreaterThanRelation, ForbiddenLessThanRelation: + forbid_a_b = forbid(a, b) - cs = ConfigurationSpace() - cs.add([a, enable_a, cond_a, b, forbid_a_b]) - cs.sample_configuration(100) + cs = ConfigurationSpace() + cs.add([a, enable_a, cond_a, b, forbid_a_b]) + cs.sample_configuration(100) From 5ebe8e43a8e3dfd4e4ccd1645f4f85a5719da23b Mon Sep 17 00:00:00 2001 From: Bogdan Budescu Date: Thu, 21 Nov 2024 08:30:04 +0200 Subject: [PATCH 5/6] rearrange some code to pass ruff formatter check --- src/ConfigSpace/forbidden.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ConfigSpace/forbidden.py b/src/ConfigSpace/forbidden.py index 0f1d5ad3..5eb535db 100644 --- a/src/ConfigSpace/forbidden.py +++ b/src/ConfigSpace/forbidden.py @@ -694,8 +694,9 @@ def is_forbidden_vector_array(self, arr: Array[f64]) -> Mask: left = arr[self.vector_ids[0]] right = arr[self.vector_ids[1]] valid = ~(np.isnan(left) | np.isnan(right)) + tmp = self.left.to_value(left[valid]) == self.right.to_value(right[valid]) out = np.zeros_like(valid) - out[valid] = self.left.to_value(left[valid]) == self.right.to_value(right[valid]) + out[valid] = tmp return out # type: ignore From 1c207f25ce3fc0a1626081e240fd5ebe0cd2b64a Mon Sep 17 00:00:00 2001 From: Bogdan Budescu Date: Thu, 21 Nov 2024 11:13:02 +0200 Subject: [PATCH 6/6] run ruff format on the entire project, not just on src subdir --- docs/scripts/api_generator.py | 1 + docs/scripts/debug_which_page_is_being_rendered.py | 2 ++ test/test_forbidden.py | 12 ++++++++---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/scripts/api_generator.py b/docs/scripts/api_generator.py index 038ed9dd..62a9048c 100644 --- a/docs/scripts/api_generator.py +++ b/docs/scripts/api_generator.py @@ -2,6 +2,7 @@ # https://mkdocstrings.github.io/recipes/ """ + from __future__ import annotations import logging diff --git a/docs/scripts/debug_which_page_is_being_rendered.py b/docs/scripts/debug_which_page_is_being_rendered.py index 5f8b642f..efbe40ed 100644 --- a/docs/scripts/debug_which_page_is_being_rendered.py +++ b/docs/scripts/debug_which_page_is_being_rendered.py @@ -3,6 +3,7 @@ This makes it easier to identify which file is being rendered when an error happens. """ + from __future__ import annotations import logging @@ -16,6 +17,7 @@ log = logging.getLogger("mkdocs") + def on_pre_page( page: mkdocs.structure.pages.Page, config: Any, diff --git a/test/test_forbidden.py b/test/test_forbidden.py index 5d287e93..7f47b881 100644 --- a/test/test_forbidden.py +++ b/test/test_forbidden.py @@ -296,12 +296,16 @@ def test_relation(): def test_relation_conditioned(): from ConfigSpace import EqualsCondition, ConfigurationSpace - a = OrdinalHyperparameter('a', [2, 5, 10]) - enable_a = CategoricalHyperparameter('enable_a', [False, True], weights=[99999, 1]) + a = OrdinalHyperparameter("a", [2, 5, 10]) + enable_a = CategoricalHyperparameter("enable_a", [False, True], weights=[99999, 1]) cond_a = EqualsCondition(a, enable_a, True) - b = OrdinalHyperparameter('b', [5, 10, 15]) - for forbid in ForbiddenEqualsRelation, ForbiddenGreaterThanRelation, ForbiddenLessThanRelation: + b = OrdinalHyperparameter("b", [5, 10, 15]) + for forbid in ( + ForbiddenEqualsRelation, + ForbiddenGreaterThanRelation, + ForbiddenLessThanRelation, + ): forbid_a_b = forbid(a, b) cs = ConfigurationSpace()