From d932ceb9d7fb6121877a3bb1ced020ed461b543c Mon Sep 17 00:00:00 2001 From: "Alexander V. Hopp" Date: Mon, 9 Dec 2024 15:34:29 +0100 Subject: [PATCH 1/6] Add test for cardinality constraints that remove all parameters of other constraints --- .../test_cardinality_constraint_continuous.py | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/constraints/test_cardinality_constraint_continuous.py b/tests/constraints/test_cardinality_constraint_continuous.py index e1da64f37..b47dce0c7 100644 --- a/tests/constraints/test_cardinality_constraint_continuous.py +++ b/tests/constraints/test_cardinality_constraint_continuous.py @@ -169,3 +169,36 @@ def test_random_recommender_with_cardinality_constraint( _validate_samples( recommendations, max_cardinality=2, min_cardinality=1, batch_size=batch_size ) + + +def test_empty_constraints_after_cardinality_constraint(): + """Constraints that have no more parameters left due to activated + cardinality constraints do not cause crashes.""" # noqa + + N_PARAMETERS = 4 + + parameters = [ + NumericalContinuousParameter(name=f"x_{i+1}", bounds=(0, 1)) + for i in range(N_PARAMETERS) + ] + constraints = [ + ContinuousLinearConstraint( + parameters=["x_1"], + operator="=", + coefficients=[1.0], + rhs=0.3, + ), + ContinuousLinearConstraint( + parameters=["x_2"], + operator="<=", + coefficients=[1.0], + rhs=0.6, + ), + ContinuousCardinalityConstraint( + parameters=["x_1", "x_2"], + max_cardinality=1, + min_cardinality=1, + ), + ] + searchspace = SearchSpace.from_product(parameters, constraints) + searchspace.continuous.sample_uniform(1) From 9f0b5b4eb5d5970a02e1ec82242fa042c2c9d9ae Mon Sep 17 00:00:00 2001 From: "Alexander V. Hopp" Date: Mon, 9 Dec 2024 15:53:22 +0100 Subject: [PATCH 2/6] Add check whether any parameters are left after dropping parameters --- baybe/searchspace/continuous.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/baybe/searchspace/continuous.py b/baybe/searchspace/continuous.py index 26447faa1..c5a96ab9b 100644 --- a/baybe/searchspace/continuous.py +++ b/baybe/searchspace/continuous.py @@ -298,10 +298,14 @@ def _drop_parameters(self, parameter_names: Collection[str]) -> SubspaceContinuo return SubspaceContinuous( parameters=[p for p in self.parameters if p.name not in parameter_names], constraints_lin_eq=[ - c._drop_parameters(parameter_names) for c in self.constraints_lin_eq + c._drop_parameters(parameter_names) + for c in self.constraints_lin_eq + if [p for p in c.parameters if p not in parameter_names] ], constraints_lin_ineq=[ - c._drop_parameters(parameter_names) for c in self.constraints_lin_ineq + c._drop_parameters(parameter_names) + for c in self.constraints_lin_ineq + if [p for p in c.parameters if p not in parameter_names] ], ) From ba79dc618b8a96427672827887f4b8df551f415c Mon Sep 17 00:00:00 2001 From: "Alexander V. Hopp" Date: Tue, 10 Dec 2024 13:05:39 +0100 Subject: [PATCH 3/6] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73e1ee5e2..d58b95d74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `qNIPV` not working with single `MIN` targets - Passing a `TargetTransformation` without passing `bounds` when createing a `NumericalTarget` now raises an error +- Crash when using ContinuousCardinalityConsraint caused by an unintended interplay + between constraints and dropped parameters ### Deprecations - Passing a dataframe via the `data` argument to `Objective.transform` is no longer From 250b9cfd1db154940d4106bc4f8d7c932e506d7c Mon Sep 17 00:00:00 2001 From: AdrianSosic Date: Wed, 11 Dec 2024 08:44:43 +0100 Subject: [PATCH 4/6] Reduce test to minimal necessary setup --- tests/constraints/test_cardinality_constraint_continuous.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/constraints/test_cardinality_constraint_continuous.py b/tests/constraints/test_cardinality_constraint_continuous.py index b47dce0c7..69aa71084 100644 --- a/tests/constraints/test_cardinality_constraint_continuous.py +++ b/tests/constraints/test_cardinality_constraint_continuous.py @@ -175,7 +175,7 @@ def test_empty_constraints_after_cardinality_constraint(): """Constraints that have no more parameters left due to activated cardinality constraints do not cause crashes.""" # noqa - N_PARAMETERS = 4 + N_PARAMETERS = 2 parameters = [ NumericalContinuousParameter(name=f"x_{i+1}", bounds=(0, 1)) @@ -200,5 +200,5 @@ def test_empty_constraints_after_cardinality_constraint(): min_cardinality=1, ), ] - searchspace = SearchSpace.from_product(parameters, constraints) - searchspace.continuous.sample_uniform(1) + subspace = SubspaceContinuous.from_product(parameters, constraints) + subspace.sample_uniform(1) From 689dc1471485f22987a805828024dd16e2ccea24 Mon Sep 17 00:00:00 2001 From: AdrianSosic Date: Wed, 11 Dec 2024 08:46:14 +0100 Subject: [PATCH 5/6] Improve readability of condition --- baybe/searchspace/continuous.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baybe/searchspace/continuous.py b/baybe/searchspace/continuous.py index c5a96ab9b..5fedf3f4e 100644 --- a/baybe/searchspace/continuous.py +++ b/baybe/searchspace/continuous.py @@ -300,12 +300,12 @@ def _drop_parameters(self, parameter_names: Collection[str]) -> SubspaceContinuo constraints_lin_eq=[ c._drop_parameters(parameter_names) for c in self.constraints_lin_eq - if [p for p in c.parameters if p not in parameter_names] + if set(c.parameters) - set(parameter_names) ], constraints_lin_ineq=[ c._drop_parameters(parameter_names) for c in self.constraints_lin_ineq - if [p for p in c.parameters if p not in parameter_names] + if set(c.parameters) - set(parameter_names) ], ) From b28db4316fbb128d74e570b525cfbbd809daaa24 Mon Sep 17 00:00:00 2001 From: AdrianSosic Date: Wed, 11 Dec 2024 08:50:38 +0100 Subject: [PATCH 6/6] Fix and extend changelog entry --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d58b95d74..f82116476 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,8 +33,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `qNIPV` not working with single `MIN` targets - Passing a `TargetTransformation` without passing `bounds` when createing a `NumericalTarget` now raises an error -- Crash when using ContinuousCardinalityConsraint caused by an unintended interplay - between constraints and dropped parameters +- Crash when using `ContinuousCardinalityConstraint` caused by an unintended interplay + between constraints and dropped parameters yielding empty parameter sets ### Deprecations - Passing a dataframe via the `data` argument to `Objective.transform` is no longer