Botorch with cardinality constraint via sampling #301

Waschenbacher · 2024-07-03T20:47:54Z

This PR adds support for cardinality constraints to BotorchRecommender. The core idea is to tackle the problem in an exhaustive search like manner, i.e. by

enumerating the possible combinations of in-/active parameters dictated by the cardinality constraints
optimizing the corresponding restricted subspaces, where the cardinality constraint can then be simply removed since the in-/active sets are fixed within these subspaces
aggregating the optimization results of the individual subspaces into a single recommendation batch.

The PR implements two mechanisms for determining the configuration of inactive parameters:

When the combinatorial list of possible inactive parameter configurations is not too large, we iterate the full list
otherwise, a fixed amount of inactive parameter configurations is randomly selected

The current aggregation step is to simply optimize all subspaces independently of each other and then return the batch from the subspace where the highest acquisition value is achieved. This has the side-effect that the set of inactive parameters is the same across the entire recommendation batch. This can be a desirable property in many use cases but potentially higher acquisition values can be obtained by altering the in-/activity sets across the batch. A simple way to achieve this (though out of scope for this PR) is by generalizing the sequential greedy principle to multiple subspaces.

Out of scope

Fulfilling cardinality constraints by passing them in suitable form as nonlinear constraints to the optimizer
Sequential greedy optimization to achieve varying in-/activity sets (see explanation above)

CHANGELOG.md

AdrianSosic

Hi @Waschenbacher, thanks for the work. This is not yet a review but only a first batch of very high-level comments that I would like you to address before we can go into the actual review process. The reason being that the main functionality brought by this PR is currently rather convoluted and hard to parse, so I'd prefer to to work with a more readable version, tbh

baybe/constraints/continuous.py

baybe/recommenders/pure/bayesian/botorch.py

baybe/searchspace/continuous.py

AdrianSosic

Hi @Waschenbacher, I've finally managed to spend some time on this important PR, thanks again for your preparation work. I've already refactored some parts that were clear to me. However, I'm not yet certain about the design at the remaining places. I have the feeling that it can potentially be simplified a lot, depending on whether it is possible to reuse the idea of reduced subspaces. Have marked the corresponding places with comments. I think we need to discuss this part first before we can continue.

baybe/searchspace/continuous.py

baybe/recommenders/pure/bayesian/botorch.py

…ctive parameters

…rch recommender

AVHopp

First high-level review regarding points that we should discuss. Not a full review yet as I think that the code might change depending on what is decided regarding the min_cardinality.

baybe/parameters/numerical.py

baybe/parameters/utils.py

AVHopp · 2024-11-04T15:45:17Z

baybe/recommenders/pure/bayesian/botorch.py

@@ -76,6 +85,13 @@ class BotorchRecommender(BayesianRecommender):
    optimization. **Does not affect purely discrete optimization**.
    """

+    max_n_subspaces: int = field(default=10, validator=[instance_of(int), ge(1)])


I think that it should be linked to cardinality somehow: For people that are not interested in cardinality constraints, it is only clear after reading the docstring that this is not interesting for them. This would however make the name quite long :/ So although I am not perfectly happy with the name, we could keep it as I do not see a better alternative while keeping it here.

AVHopp · 2024-11-04T15:46:39Z

baybe/recommenders/pure/bayesian/botorch.py

+        subspace_continuous: SubspaceContinuous,
+        batch_size: int,
+    ) -> tuple[Tensor, Tensor]:
+        """Recommend from a continuous search space with cardinality constraints.


I like it, only some information about how max_n_subspaces comes into play here is still missing for me :)

baybe/recommenders/pure/bayesian/botorch.py

AVHopp · 2024-11-04T15:49:23Z

baybe/searchspace/continuous.py

@@ -301,6 +334,45 @@ def _drop_parameters(self, parameter_names: Collection[str]) -> SubspaceContinuo
            ],
        )

+    def _enforce_cardinality_constraints_via_assignment(


Since there is only one way of enforcing cardinality consraints, why not simply enforce_cardinality_constraints? If people are interested in the details of the how, they can read the docstring.

- Replace redandunt function - Ensure near-zero range being an open interval

- Add to-do related to customized error in botorch - Add to-do related to active parameters guarantee in random sampler

…ity constraint

…a_sampling

Waschenbacher · 2024-12-16T13:53:41Z

@AdrianSosic This is the updated PR according to our discussion in the baybathon. The main changes are below:

A threshold near_zero_threshold is added to each parameter NumericalContinuousParameter. This implementation deviates from the original plan of
- Threshold for each constraint or subspace vs threshold for each parameter. I decide for a threshold for one parameter because I think this way is more flexible and more logical
- Threshold ratio (the original suggestion) vs threshold. I decide for threshold since the logic gets complicated with ratio of threshold and there are several doubts when I want to infer the absolute threshold from the ratio of threshold:
- - Is the threshold ratio based on the complete region or just one side?
- - If the bounds are not symmetrical, e.g. a parameter has bounds (a, b)with a!=b. How to infer the absolute threshold values? Do we want to keep a single absolute threshold or two different numbers?
- - How to deal with infinite bounds?
- - The user can infer the desired threshold from any threshold ratio, if needed, and assign them.
Check whether any minimum cardinality constraint is violated in botorch's recommendation and raise the warning MinimumCardinalityViolatedWarning if it occurs.
Added a test catching the MinimumCardinalityViolatedWarning is raised when any minimum cardinality constraint is violated.
Update the test related on cardinality constraint: all maximum cardinality constraints are fulfilled and either all minimum cardinality constraints are fulfilled or a warning is raised if not.
Created a botorch PR (Add InfeasibilityError exception pytorch/botorch#2652) for the dangerous ValueError due to infeasibility and added TODO.

baybe/constraints/continuous.py

baybe/parameters/numerical.py

baybe/parameters/utils.py

Scienfitz · 2024-12-19T08:29:52Z

baybe/constraints/validation.py

@@ -41,6 +49,9 @@ def validate_constraints(  # noqa: DOC101, DOC103
    param_names_discrete = [p.name for p in parameters if p.is_discrete]
    param_names_continuous = [p.name for p in parameters if p.is_continuous]
    param_names_non_numerical = [p.name for p in parameters if not p.is_numerical]
+    params_continuous: list[NumericalContinuousParameter] = [
+        p for p in parameters if isinstance(p, NumericalContinuousParameter)


sry for my confusion, the method exists in this PR https://github.com/emdgroup/baybe/pull/291/files#diff-9b02c8d8e9e86b086ea306806ebc5e47435ac5045557f8eb138a7be22c3cb0e8R461 which is however on hold

AVHopp

This is an incomplete review as I was not aware that there is still stuff being worked on. Feel free to either already include my comments or just resolve them.

baybe/parameters/numerical.py

AVHopp · 2025-01-08T13:24:42Z

baybe/parameters/numerical.py

+
+        Important:
+            Value in the open interval (-near_zero_threshold, near_zero_threshold)
+            will be treated as near_zero.


Insonsistencies regarding the use of near_zero and near-zero in this docstring.

It is renamed to zeros in the updated version. Resolve it for now. If near_zero is preferred, I'm open to rename it back.

baybe/parameters/numerical.py

baybe/parameters/utils.py

AVHopp · 2025-01-08T13:41:17Z

CHANGELOG.md

+  cardinality is violated in `BotorchRecommender`
+- Attribute `max_n_subspaces` to `BotorchRecommender`, allowing to control
+  optimization behavior in the presence of multiple subspaces
+- Utilities `inactive_parameter_combinations` and`n_inactive_parameter_combinations` 


I think these as well as the utilities noted below are not user-facing, are they? In that case, I do not think that it is necessary to include them in the CHANGELOG

AVHopp · 2025-01-08T13:45:46Z

baybe/recommenders/pure/bayesian/botorch.py

+
+        return pd.DataFrame(points, columns=subspace_continuous.parameter_names)
+
+    def _recommend_continuous_torch(


Why do we need an additional function for that? In my opinion, this check could be done in the original function, and this function just adds another layer of complexity. Also, I think the name is weird, why the explicit mention of torch?

_recommend_continuous is the outer layer and does some checks and returns pd.DataFrame, while _recommend_continuous_torch is only responsible of returning points, acqf_value with type tensor. Moreover, _recommend_continuous_torch is needed in _optimize_continuous_subspaces (see https://github.com/emdgroup/baybe/pull/301/files#r1825774190). Correct me if I'm wrong @AdrianSosic

Yes, exactly, it's to fit the required interfaces on both sides. _recommend_continuous still operates on the dataframe level as it needs to return outputs that are then shipped to the user. But there are also places like _optimize_continuous_subspaces where you still need access to the low-level output like the corresponding acqf values. The only (sort of reasonable) way I saw to achieve both here is to extract that inner part and declare a separate function for it. But I'd be very happy if you see a more elegant alternative 👍🏼

AVHopp · 2025-01-08T13:47:02Z

baybe/recommenders/pure/bayesian/botorch.py

+        subspace_continuous: SubspaceContinuous,
+        batch_size: int,
+    ) -> tuple[Tensor, Tensor]:
+        """Recommend from a continuous search space with cardinality constraints.


This is still the case.

baybe/recommenders/pure/bayesian/botorch.py

- Support checking minimum cardinality or maximum cardinatliy - Adapt to threshold per cardinality - Update related tests

… parameter

* Assure parameter bounds cover zero * Check invalid "activate_parameter" option first

AVHopp · 2025-01-17T16:37:07Z

Since there seems to be the wish to merge this soon, please put this out of draft mode if it is indeed ready for review. I will not have a look a this earlier, since I assume that being in Draft mode means that it is not yet ready (which however conflicts with some of the comments that I see here)

Waschenbacher added the new feature New functionality label Jul 3, 2024

Waschenbacher self-assigned this Jul 3, 2024

Waschenbacher requested review from Scienfitz, AdrianSosic and AVHopp as code owners July 3, 2024 20:47

Waschenbacher marked this pull request as draft July 4, 2024 07:38

Waschenbacher force-pushed the feature/cardinality_constraint_to_botorch_via_sampling branch from 0be3285 to 1f7783d Compare July 4, 2024 09:21

Waschenbacher marked this pull request as ready for review July 4, 2024 09:35

AVHopp reviewed Jul 5, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

AdrianSosic requested changes Jul 9, 2024

View reviewed changes

Waschenbacher force-pushed the feature/cardinality_constraint_to_botorch_via_sampling branch from 1f7783d to 9d28b49 Compare July 12, 2024 07:38

AdrianSosic force-pushed the feature/cardinality_constraint_to_botorch_via_sampling branch from f68ce4f to fc52875 Compare August 15, 2024 12:52

AdrianSosic requested changes Aug 15, 2024

View reviewed changes

baybe/searchspace/continuous.py Outdated Show resolved Hide resolved

baybe/recommenders/pure/bayesian/botorch.py Show resolved Hide resolved

Waschenbacher and others added 17 commits August 26, 2024 10:29

Enable cardinality constraint in botorch recommender via sampling ina…

d811240

…ctive parameters

Make inactive parameters fixed features

da813f5

Fix bug in test file

adf5cc2

Validate bounds of cardinality constraint parameters

e69ceff

Add second option: iterate through combinatorial list

2270350

Fix type error

6483e4b

Revise botorch+cardinality constraint for enhanced clarity

ae919d4

Fix property names and its docstrings

9ab8fda

Use guard clause

c3831c7

Simplify syntax with 'prod'

2f49f5a

Refactor botorch+cardinality constraint

d46fd60

Make 'n_threshold_inactive_parameters_generator' an attribute of boto…

293e2ef

…rch recommender

Refactor combinatorial properties of cardinality constraint

f8d0713

Refactor combinatorial properties of continuous subspace

76b5d72

Refactor constraint validation

5c92079

Move factory code up

f07a452

Simplify constructor code

c5b014d

AVHopp reviewed Nov 4, 2024

View reviewed changes

Scienfitz marked this pull request as draft November 26, 2024 12:48

Waschenbacher added 10 commits December 13, 2024 11:51

Add near-zero threshold to continuous numerical parameter

fa13267

Refine activate parameter helper function

7aa7c3f

- Replace redandunt function - Ensure near-zero range being an open interval

Show warnings when any minimum cardinality constraints are violated.

cfdf1e3

Update test related to cardinality constraints

e3c6620

Add to-dos

b85924f

- Add to-do related to customized error in botorch - Add to-do related to active parameters guarantee in random sampler

Add test on catching warning related to violation of minimum cardinal…

22b19f9

…ity constraint

Update CHANGELOG.md

b0dc037

Merge branch 'main' into feature/cardinality_constraint_to_botorch_vi…

3fa8b02

…a_sampling

Clean up merge conflict code

35825b8

Refine logic in counting the near-zero elements

3e275e4

Scienfitz reviewed Dec 19, 2024

View reviewed changes

Add TODO related to customized infeasibility error in botorch

62f0ed6

AVHopp reviewed Jan 8, 2025

View reviewed changes

Waschenbacher added 4 commits January 13, 2025 21:06

Add threshold to continuous cardinality constraint

9af846b

Adapt activate_parameter towards threshold per cardinality constraints

10e0812

Refine check cardinaltiy constraint fulfillment logic

142b1ec

- Support checking minimum cardinality or maximum cardinatliy - Adapt to threshold per cardinality - Update related tests

Remove threshold related attribute and method in numerical continuous…

04f145c

… parameter

Waschenbacher force-pushed the feature/cardinality_constraint_to_botorch_via_sampling branch from ee54c89 to bb1fc3d Compare January 13, 2025 21:42

Waschenbacher added 7 commits January 14, 2025 10:45

Make zero-checking and threshold definition compatible

55a7ba3

* Assure parameter bounds cover zero * Check invalid "activate_parameter" option first

Add activate parameter step in random sampler

78b115f

Update CHANGELOG.md

68045a7

Fix type hint in continuous numerical parameter classes

e6e2e97

Test activate parameter function

a30b009

Correct logic on boundary handling in activate paramter

983b1a9

Ensure parameter bounds cover zero

bddab62

Waschenbacher force-pushed the feature/cardinality_constraint_to_botorch_via_sampling branch from bb1fc3d to bddab62 Compare January 14, 2025 09:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Botorch with cardinality constraint via sampling #301

Botorch with cardinality constraint via sampling #301

Waschenbacher commented Jul 3, 2024 •

edited by AdrianSosic

Loading

AdrianSosic left a comment

AdrianSosic left a comment

AVHopp left a comment

AVHopp Nov 4, 2024

AVHopp Nov 4, 2024

AVHopp Nov 4, 2024

Waschenbacher commented Dec 16, 2024

Scienfitz Dec 19, 2024

AVHopp left a comment

AVHopp Jan 8, 2025

Waschenbacher Jan 9, 2025

AVHopp Jan 8, 2025

AVHopp Jan 8, 2025

Waschenbacher Jan 14, 2025

AdrianSosic Jan 17, 2025

AVHopp Jan 8, 2025

AVHopp commented Jan 17, 2025


		return pd.DataFrame(points, columns=subspace_continuous.parameter_names)

		def _recommend_continuous_torch(

Botorch with cardinality constraint via sampling #301

Are you sure you want to change the base?

Botorch with cardinality constraint via sampling #301

Conversation

Waschenbacher commented Jul 3, 2024 • edited by AdrianSosic Loading

Out of scope

AdrianSosic left a comment

Choose a reason for hiding this comment

AdrianSosic left a comment

Choose a reason for hiding this comment

AVHopp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Waschenbacher commented Dec 16, 2024

Choose a reason for hiding this comment

AVHopp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AVHopp commented Jan 17, 2025

Waschenbacher commented Jul 3, 2024 •

edited by AdrianSosic

Loading