Continuous inter-point constraints #345

AVHopp · 2024-08-20T06:49:25Z

This PR introduces a first variant of inter-point constraints by using the botorch-provided interface.

Here, an inter-point constraint is a constraint that acts on a full batch instead of a single recommendation. If we think of a batch recommendation of a matrix with shape batches x features, then our previous constraints would be row-wise, while these constraints allow mixed constraints across both dimensions.

This PR introduces new classes, and these classes were modeled similar to the already existing classes for continuous constraints. Some things had to be changed though resp. we need to aware:

Most importantly, the way of sampling points from a polytope. The reason is that get_polytope_samples is not made for including inter-point constraints, and hence a workaround was implemented. The workaround basically transforms the space in a one-dimensional space with batches * features many features, and then defines both normal and inter-point constraints over this space (see [Bug] get_polytope_samples fails for inequalities over q-batches and the actual dimension pytorch/botorch#2468). Note that this might interfere with Botorch with cardinality constraint via sampling #301 and that some alignment might be necessary.
The constraints in this PR always combines full columns. That is, the constraints $x_1 <=1$ would be interpreted as "the sum of $x_1$ across the whole batch needs to be smaller than 1". This can e.g. be used to limit the usage of resources, like only having 100ml of a substance available for the full batch. Also, different columns can be combined, so having $x_2 + 2*x_3 <=100$ would mean "sum of $x_1$ plus two times the sum of $x_3$ across the batch has to be smaller than 100$.
For now, the constraints have only been applied for continuous search spaces. The reason for this is that I want to keep this PR rather short and use it to discuss in general about the design, and to avoid any potential conflicts with Botorch with cardinality constraint via sampling #301 . However, including them in hybrid search spaces should be possible already in this PR. For discrete spaces, this still needs some investigation. So interpret this as just a first step in the direction of more :)

AVHopp · 2024-08-21T06:42:31Z

Note: This PR is currently on hold for two reasons:

There was a misunderstanding between @AdrianSosic and me what "inter-point" constraint actually means, and incorporating the necessary changes might require some more redesign.
There is another open PR which currently changes the behavior of get_polytope_samples, and that PR should be merged first.

CHANGELOG.md

Scienfitz · 2024-10-31T16:46:39Z

as mentioned in our call but posting here for documentation / reference: I think its worth exploring whether a design not via deriving from ContinuousLinearConstraint but by adding a simple switching flag to it might be better

AdrianSosic

Not yet completely done with the review, but because time is running, here a first batch of comments

AdrianSosic · 2024-11-12T12:51:17Z

baybe/recommenders/pure/bayesian/botorch.py

@@ -234,6 +234,8 @@ def _recommend_hybrid(
        Returns:
            The recommended points.
        """
+        # TODO Interpoint constraints are not yet enabled in hybrid search spaces


Can you raise an actual NotImplementedError here? Otherwise, these constraints would just silently be ignored.

AdrianSosic · 2024-11-12T12:52:16Z

CHANGELOG.md

General comment: through the PR, the writing is not consistent. Sometimes you write interpoint and sometimes inter-point. Let's settle for one and be consistent.

AdrianSosic · 2024-11-12T12:55:52Z

baybe/searchspace/continuous.py

@@ -82,6 +82,10 @@ def __str__(self) -> str:
        nonlin_constraints_list = [
            constr.summary() for constr in self.constraints_nonlin
        ]
+        nonlin_constraints_list = [


duplication

AdrianSosic · 2024-11-12T12:57:42Z

baybe/searchspace/continuous.py

+    @property
+    def is_constrained(self) -> bool:
+        """Return whether the subspace is constrained in any way."""
+        return any(
+            (
+                self.constraints_lin_eq,
+                self.constraints_lin_ineq,
+                self.constraints_nonlin,
+            )
+        )
+
+    @property
+    def has_interpoint_constraints(self) -> bool:
+        """Return whether or not the space has any interpoint constraints."""
+        return any(
+            c.is_interpoint for c in self.constraints_lin_eq + self.constraints_lin_ineq
+        )
+


Docstrings of properties should read as if they were an attribute, i.e. no verb like return .... Instead, something like Boolean flag indicating ...

AdrianSosic · 2024-11-12T12:58:22Z

baybe/searchspace/continuous.py

@@ -359,7 +381,10 @@ def samples_random(self, n_points: int = 1) -> pd.DataFrame:
        )
        return self.sample_uniform(n_points)

-    def sample_uniform(self, batch_size: int = 1) -> pd.DataFrame:
+    def sample_uniform(


what's this reformat about? By mistake?

AdrianSosic · 2024-11-12T13:01:13Z

baybe/constraints/continuous.py

@@ -45,6 +46,15 @@ class ContinuousLinearConstraint(ContinuousConstraint):
    rhs: float = field(default=0.0, converter=float, validator=finite_float)
    """Right-hand side value of the in-/equality."""

+    is_interpoint: bool = field(default=False)


Suggested change

is_interpoint: bool = field(default=False)

is_interpoint: bool = field(default=False, validator=instance_of(bool))

AdrianSosic · 2024-11-12T13:16:59Z

baybe/constraints/continuous.py

@@ -45,6 +46,15 @@ class ContinuousLinearConstraint(ContinuousConstraint):
    rhs: float = field(default=0.0, converter=float, validator=finite_float)
    """Right-hand side value of the in-/equality."""

+    is_interpoint: bool = field(default=False)


Not sure what is more in line with our convention, is_interpoint or interpoint 🤔 The former is what we typically use for properties, while the latter makes a bit more sense from a constructor perspective. @Scienfitz: opinions?

I agree, would use the latter for arguments

AdrianSosic · 2024-11-12T13:21:13Z

baybe/constraints/continuous.py

+    An inter-point constraint is a constraint that is defined over full batches. That
+    is, and inter-point constraint of the form ``param_1 + 2*param_2 <=2`` means that
+    the sum of ``param2`` plus two times the sum of ``param_2`` across the full batch
+    must not exceed 2.


Suggested change

An inter-point constraint is a constraint that is defined over full batches. That

is, and inter-point constraint of the form ``param_1 + 2*param_2 <=2`` means that

the sum of ``param2`` plus two times the sum of ``param_2`` across the full batch

must not exceed 2.

While intra-point constraints impose conditions on each individual point of a batch,

inter-point constraints do so **across** the points of the batch. That is, an

inter-point constraint of the form ``x_1 + x_2 <= 1`` enforces that the sum of all

``x_1`` values plus the sum of all ``x_2`` values in the batch must not exceed 1.

CHANGELOG.md

Scienfitz · 2024-11-15T08:25:53Z

baybe/constraints/continuous.py

@@ -45,6 +46,15 @@ class ContinuousLinearConstraint(ContinuousConstraint):
    rhs: float = field(default=0.0, converter=float, validator=finite_float)
    """Right-hand side value of the in-/equality."""

+    is_interpoint: bool = field(default=False)


I agree, would use the latter for arguments

Scienfitz · 2024-11-15T08:31:13Z

baybe/constraints/continuous.py

+        self,
+        parameters: Sequence[NumericalContinuousParameter],
+        idx_offset: int = 0,
+        batch_size: int = 1,


i wouldnt give this an int default ('.recommend' also does not have a default for batch size) but would make the default 'None'

Scienfitz · 2024-11-15T08:35:06Z

examples/Constraints_Continuous/linear_constraints.py

@@ -18,7 +18,9 @@
 from botorch.test_functions import Rastrigin

 from baybe import Campaign
-from baybe.constraints import ContinuousLinearConstraint
+from baybe.constraints import (
+    ContinuousLinearConstraint,


strange reformat

Scienfitz · 2024-11-15T08:36:43Z

examples/Constraints_Continuous/linear_constraints.py

@@ -140,3 +142,59 @@
    "2.0*x_2 + 3.0*x_4 <= 1.0 satisfied in all recommendations? ",
    (2.0 * measurements["x_2"] + 3.0 * measurements["x_4"]).le(1.0 + TOLERANCE).all(),
 )
+
+
+### Using inter-point constraints


interpoint should be its own example file feels just 'drangepappt' for this example

Scienfitz · 2024-11-15T08:48:15Z

examples/Constraints_Continuous/linear_constraints.py

+# Since these constraints require information about the batch size, they are not used
+# during the creation of the search space but handed over to the `recommend` call.
+# This example models the following inter-point constraints and combines them also
+# with regular constraints.


that does not appear to be true

Scienfitz · 2024-11-15T08:49:06Z

examples/Constraints_Continuous/linear_constraints.py

+    rec["Target"] = target_values
+    inter_campaign.add_measurements(rec)
+    # Check inter-point constraints
+    assert rec["x_1"].sum() >= 2.5 - TOLERANCE


prints are more important than asserts in examples (nbut nothing speaks against having both)

Scienfitz · 2024-11-15T08:59:59Z

baybe/searchspace/continuous.py

+        for c in [*self.constraints_lin_eq, *self.constraints_lin_ineq]:
+            if not c.is_interpoint:
+                param_indices, coefficients, rhs = c.to_botorch(
+                    self.parameters, batch_size=batch_size


Suggested change

self.parameters, batch_size=batch_size

self.parameters

Scienfitz · 2024-11-15T09:03:36Z

baybe/searchspace/continuous.py

            return self._sample_from_bounds(batch_size, self.comp_rep_bounds.values)

+        if self.has_interpoint_constraints:
+            return self._sample_from_polytope_with_interpoint_constraints(


is it feasible to keep splittitng the sample function up like that? eg what happens if we have interpoint+cardinality constraints

Scienfitz · 2024-11-15T09:05:36Z

baybe/searchspace/continuous.py

            return self._sample_from_bounds(batch_size, self.comp_rep_bounds.values)

+        if self.has_interpoint_constraints:
+            return self._sample_from_polytope_with_interpoint_constraints(


isnt 'sample_from_polytope' a special case of the new function? If so I think it could be better design if one was contained int he other or one calls the other

Scienfitz · 2024-11-15T09:07:10Z

CHANGELOG.md

the new intperpoint flag should be mentioned in the userguide

AVHopp added the new feature New functionality label Aug 20, 2024

AVHopp self-assigned this Aug 20, 2024

AVHopp requested review from Scienfitz and AdrianSosic as code owners August 20, 2024 06:49

AVHopp marked this pull request as draft August 20, 2024 12:36

AVHopp added the on hold PR progress is awaiting for something else to continue label Aug 21, 2024

AVHopp force-pushed the feature/interpoint_constraints branch from a5ed090 to 7cbb490 Compare September 18, 2024 11:47

AVHopp removed the on hold PR progress is awaiting for something else to continue label Sep 18, 2024

AVHopp marked this pull request as ready for review September 18, 2024 11:48

AVHopp commented Sep 18, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

AVHopp force-pushed the feature/interpoint_constraints branch from 7cbb490 to d5758c5 Compare September 27, 2024 13:51

AVHopp commented Sep 27, 2024

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

AVHopp marked this pull request as draft September 30, 2024 12:13

AVHopp force-pushed the feature/interpoint_constraints branch from d5758c5 to ecb050e Compare October 22, 2024 10:58

AVHopp marked this pull request as ready for review October 22, 2024 11:35

AVHopp force-pushed the feature/interpoint_constraints branch from e82a4cc to 1e2cd40 Compare November 11, 2024 17:01

AVHopp added 11 commits November 11, 2024 18:02

Introduce class for continuous interpoint constraints

4954a15

Add interpoint constraints to search space

79057de

Adjust sampling for interpoint constraints

bcb8f66

Use interpoint constraints in BotorchRecommender

a702b46

Include interpoint constraints in example

a63ac1f

Include interpoint constraints in tests

05f4516

Fix to_botorch mypy error

34c7a11

Fix inorrect type hint

174bfe6

Rename and reshuffle to make mypyhappy

f8352a4

Simplify definition of inter-point constraints

94d99b7

Update CHANGELOG

004ec49

AVHopp force-pushed the feature/interpoint_constraints branch from 1e2cd40 to 004ec49 Compare November 11, 2024 17:02

Fix mypy issue

2aaab58

AdrianSosic requested changes Nov 12, 2024

View reviewed changes

Scienfitz requested changes Nov 15, 2024

View reviewed changes

CHANGELOG.md

Copy link

Collaborator

Scienfitz Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new intperpoint flag should be mentioned in the userguide

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Continuous inter-point constraints #345

Continuous inter-point constraints #345

AVHopp commented Aug 20, 2024 •

edited

Loading

AVHopp commented Aug 21, 2024

Scienfitz commented Oct 31, 2024

AdrianSosic left a comment

AdrianSosic Nov 12, 2024

AdrianSosic Nov 12, 2024

AdrianSosic Nov 12, 2024

AdrianSosic Nov 12, 2024

AdrianSosic Nov 12, 2024

AdrianSosic Nov 12, 2024

AdrianSosic Nov 12, 2024

Scienfitz Nov 15, 2024

AdrianSosic Nov 12, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

Scienfitz Nov 15, 2024

	is_interpoint: bool = field(default=False)
	is_interpoint: bool = field(default=False, validator=instance_of(bool))

Continuous inter-point constraints #345

Are you sure you want to change the base?

Continuous inter-point constraints #345

Conversation

AVHopp commented Aug 20, 2024 • edited Loading

AVHopp commented Aug 21, 2024

Scienfitz commented Oct 31, 2024

AdrianSosic 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

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

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 Aug 20, 2024 •

edited

Loading