Enabling candidate generation with both `non_linear_constraints` and `fixed_features` #1912

jduerholt · 2023-07-03T13:55:54Z

Motivation

This PR adds the possibility to use fixed_features in combination with nonlinear_inequality_constraints as discussed in issue #1904.

@Balandat: concerning the solution that you suggested: I think it is much easier as the f_np_wrapper is also used for the nonlinear constraints:

botorch/botorch/generation/gen.py

Line 224 in 38e2027

f_np_wrapper=f_np_wrapper,

and fix_features is already applied there:

botorch/botorch/generation/gen.py

Line 189 in 38e2027

X_fix = fix_features(X, fixed_features=fixed_features)

So no need for another wrapper, or?

A wrapper function would be only needed if one wants to have the possibility to use the nonlinear constraints also with the reduced domain, then one would need a method, which takes the reduced X as input and augments it with the fixed features.

Here is some example code, that shows that it works:

import torch
from botorch.models import SingleTaskGP
from botorch.fit import fit_gpytorch_mll
from botorch.utils import standardize
from gpytorch.mlls import ExactMarginalLogLikelihood
from botorch.utils.sampling import get_polytope_samples

def constraint(X):
    return torch.sum(X[...,:3], dim=-1) -1.80

train_X = torch.rand(10, 3)
Y = 1 - torch.norm(train_X[:,:2] - 0.5, dim=-1, keepdim=True)
Y = Y + 0.1 * torch.randn_like(Y)  # add some noise
train_Y = standardize(Y)

gp = SingleTaskGP(train_X, train_Y)
mll = ExactMarginalLogLikelihood(gp.likelihood, gp)
fit_gpytorch_mll(mll)

from botorch.acquisition import UpperConfidenceBound

UCB = UpperConfidenceBound(gp, beta=0.1)

from botorch.optim import optimize_acqf


bounds = torch.stack([torch.zeros(3), torch.ones(3)])

batch_initial_conditions = get_polytope_samples(
    n=5,
    bounds=bounds,
    inequality_constraints=[(torch.tensor([0,1,2]),torch.tensor([1.,1.,1.]),1.8)],
    n_burnin=1000
).unsqueeze(-2)

candidate, acq_value = optimize_acqf(
    UCB, 
    bounds=bounds, 
    q=1, 
    num_restarts=5, 
    raw_samples=20,
    fixed_features={2:0.5}, 
    nonlinear_inequality_constraints = [constraint],
    batch_initial_conditions = batch_initial_conditions
)
print(candidate)  # tensor([0.4887, 0.5063])
print(candidate.sum())

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Unit tests.

codecov · 2023-07-03T14:06:58Z

Codecov Report

Merging #1912 (dfa9b64) into main (38e2027) will not change coverage.
The diff coverage is 100.00%.

❗ Current head dfa9b64 differs from pull request most recent head 0e71cdb. Consider uploading reports for the commit 0e71cdb to get more accurate results

@@            Coverage Diff            @@
##              main     #1912   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          173       173           
  Lines        15232     15290   +58     
=========================================
+ Hits         15232     15290   +58

Impacted Files	Coverage Δ
botorch/models/contextual.py	`100.00% <ø> (ø)`
botorch/models/pairwise_gp.py	`100.00% <ø> (ø)`
botorch/models/utils/assorted.py	`100.00% <ø> (ø)`
botorch/generation/gen.py	`100.00% <100.00%> (ø)`
botorch/generation/utils.py	`100.00% <100.00%> (ø)`
botorch/models/kernels/contextual_lcea.py	`100.00% <100.00%> (ø)`
botorch/models/likelihoods/pairwise.py	`100.00% <100.00%> (ø)`
botorch/optim/parameter_constraints.py	`100.00% <100.00%> (ø)`
botorch/utils/probability/utils.py	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Balandat

Yeah I think this makes sense. If there are multiple fixed features it's going to be slower than optimizing over the reduced domain since the optimizer needs to (i) identify the fixed features and (ii) SLSQP scales pretty poorly with the problem dimension anyway.

But that's probably fine for now (maybe add this comment in code for future reference?

tensor([0.4887, 0.5063])

This doesn't seem to be the right output? Should be 3dim with the third feature fixed to 0.5?

test/optim/test_optimize.py

Balandat · 2023-07-03T14:17:20Z

test/optim/test_optimize.py

+                nonlinear_inequality_constraints=[nlc1, nlc2, nlc3, nlc4],
+                batch_initial_conditions=batch_initial_conditions,
+                num_restarts=num_restarts,
+                fixed_features={0: 2},


Hmm so with this setup this test may pass even if fixing the features doesn't work (given that it only narrows this to a specific solution of the previous solution set). Can you use a different value here? I might also make sense to use a different mock acquisition function so that the optimizer is unique...

I will have a deeper look to the test. All this mocking is sometimes a bit hard for me to understand ;)

I don't think there is actually any mocking going on here - it's just that mock_acq_function is a SquaredAcquisitionFunction rather than a true acquisition function.

If you don't want to change that, one thing you could do would be to just replace one of the constraints to result in a different optimum / optimizer.

Balandat · 2023-07-03T14:22:50Z

Test failure seems unrelated I'll take a look at that

jduerholt · 2023-07-03T14:24:04Z

Yeah I think this makes sense. If there are multiple fixed features it's going to be slower than optimizing over the reduced domain since the optimizer needs to (i) identify the fixed features and (ii) SLSQP scales pretty poorly with the problem dimension anyway.

I am already working on the proper solution with the reduced domain. I will give it a few more minutes and if I think it is feasible without taking too much time, I will add it to the PR.

This doesn't seem to be the right output? Should be 3dim with the third feature fixed to 0.5?

You care correct, this is copied from the botorch website minimal example

and I forgot to remove it :D

Co-authored-by: Max Balandat <Balandat@users.noreply.github.com>

…nto non_linear_and_fixed

jduerholt · 2023-07-03T21:53:19Z

Hi @Balandat,

I implemented now also the solution using the reduced domain. Which one do you prefer?

Best,

Johannes

Balandat

Looks great, just a couple of minor style nits. Thanks for putting this up!

botorch/optim/parameter_constraints.py

test/optim/test_parameter_constraints.py

Co-authored-by: Max Balandat <Balandat@users.noreply.github.com>

jduerholt · 2023-07-04T13:37:32Z

@Balandat, I incorporated your changes and added another test which was still missing (from my perspective). From my side, this is now ready for merge.

Balandat · 2023-07-04T13:59:37Z

Great, thanks you. I'll pull it in and merge it.

facebook-github-bot · 2023-07-04T13:59:55Z

@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

SebastianAment · 2023-07-06T10:58:14Z

Test failure seems unrelated I'll take a look at that

This was solved by #1919, should pass upon re-running the tests.

SebastianAment

Thanks for adding this @jduerholt! Mainly left cosmetic suggestions that will aid readability, as well as completed type signatures.

Last, could you rename the PR something a little more descriptive like "Enabling candidate generation with both non_linear_constraints and fixed_features"? This will make it easier to understand what's going on when browsing the commit history on a high level.

SebastianAment · 2023-07-06T11:16:35Z

botorch/optim/parameter_constraints.py

+    constraints: Optional[List[Callable]],
+    fixed_features: Dict[int, float],
+    dimension: int,
+) -> Optional[List[Callable]]:


Two subjective suggestions to make this quicker to parse:

How about _gen_nonlin_constraints_of_variable_features? Could apply same convention to the linear case below. (cc @Balandat)

Could you add a doc-string here, something like "Given a dictionary of fixed_features, returns a list of non-linear constraints on the variable (non-fixed) features."

I added a docsting. Concerning renaming: should I do it, or should it be done in a seperate PR for both methods (nonlinear and linear) to keep it consistent?

Thank you! If you're up for it, you can do both in this commit. The only mentions of _generate_unfixed_lin_constraints are in:

generation/utils.py,

optim/parameter_constraints.py, and

test/optim/parameter_constraints.py.

A quick search and replace in these files should do it.

Hmm, but do you really prefer the new name? What do you do not like about _generate_unfixed_lin_constraints?

But ofc, I can do it ;)

_generate_unfixed_(non)lin_constraints makes it sound like the constraints are "unfixed", whereas it is the features that are. But let's keep it then, especially since the docstring makes it clear now. Thanks for adding it!

botorch/optim/parameter_constraints.py

botorch/generation/utils.py

Co-authored-by: Sebastian Ament <56886879+SebastianAment@users.noreply.github.com>

facebook-github-bot · 2023-07-06T13:24:25Z

@SebastianAment has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-07-06T14:29:02Z

@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2023-07-06T20:04:58Z

@Balandat merged this pull request in 6f98187.

jduerholt added 2 commits July 3, 2023 15:37

remove guarding for nonlinear constraints

8026af1

remove failing test

f86c27e

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 3, 2023

add test

3cf28b1

Balandat reviewed Jul 3, 2023

View reviewed changes

jduerholt and others added 4 commits July 3, 2023 16:25

Update test/optim/test_optimize.py

a4445be

Co-authored-by: Max Balandat <Balandat@users.noreply.github.com>

implement reduced

f30d81e

Merge branch 'non_linear_and_fixed' of github.com:jduerholt/botorch i…

82bcab1

…nto non_linear_and_fixed

fix flake8

e4346ce

fix bug

2593b81

jduerholt requested a review from Balandat July 3, 2023 22:07

fix test

d550f15

Balandat approved these changes Jul 4, 2023

View reviewed changes

jduerholt and others added 4 commits July 4, 2023 07:52

Apply suggestions from code review

fa4d4cb

Co-authored-by: Max Balandat <Balandat@users.noreply.github.com>

black it

2047e40

flake it

fe9eb1a

add test

2e177c9

jduerholt requested a review from Balandat July 4, 2023 13:37

SebastianAment reviewed Jul 6, 2023

View reviewed changes

Apply suggestions from code review

9d3f189

Co-authored-by: Sebastian Ament <56886879+SebastianAment@users.noreply.github.com>

jduerholt changed the title ~~Non linear and fixed~~ Enabling candidate generation with both non_linear_constraints and fixed_features Jul 6, 2023

fix linting

cbe0d2b

add docstring

0e71cdb

Balandat approved these changes Jul 6, 2023

View reviewed changes

facebook-github-bot closed this in 6f98187 Jul 6, 2023

facebook-github-bot added the Merged label Jul 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabling candidate generation with both `non_linear_constraints` and `fixed_features` #1912

Enabling candidate generation with both `non_linear_constraints` and `fixed_features` #1912

jduerholt commented Jul 3, 2023 •

edited

Loading

codecov bot commented Jul 3, 2023 •

edited

Loading

Balandat left a comment

Balandat Jul 3, 2023

jduerholt Jul 3, 2023

Balandat Jul 3, 2023 •

edited

Loading

Balandat commented Jul 3, 2023

jduerholt commented Jul 3, 2023

jduerholt commented Jul 3, 2023

Balandat left a comment

jduerholt commented Jul 4, 2023

Balandat commented Jul 4, 2023

facebook-github-bot commented Jul 4, 2023

SebastianAment commented Jul 6, 2023

SebastianAment left a comment

SebastianAment Jul 6, 2023

jduerholt Jul 6, 2023

SebastianAment Jul 6, 2023 •

edited

Loading

jduerholt Jul 6, 2023

jduerholt Jul 6, 2023

SebastianAment Jul 6, 2023

facebook-github-bot commented Jul 6, 2023

facebook-github-bot commented Jul 6, 2023

facebook-github-bot commented Jul 6, 2023

Enabling candidate generation with both non_linear_constraints and fixed_features #1912

Enabling candidate generation with both non_linear_constraints and fixed_features #1912

Conversation

jduerholt commented Jul 3, 2023 • edited Loading

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

codecov bot commented Jul 3, 2023 • edited Loading

Codecov Report

Balandat left a comment

Choose a reason for hiding this comment

Balandat Jul 3, 2023

Choose a reason for hiding this comment

jduerholt Jul 3, 2023

Choose a reason for hiding this comment

Balandat Jul 3, 2023 • edited Loading

Choose a reason for hiding this comment

Balandat commented Jul 3, 2023

jduerholt commented Jul 3, 2023

jduerholt commented Jul 3, 2023

Balandat left a comment

Choose a reason for hiding this comment

jduerholt commented Jul 4, 2023

Balandat commented Jul 4, 2023

facebook-github-bot commented Jul 4, 2023

SebastianAment commented Jul 6, 2023

SebastianAment left a comment

Choose a reason for hiding this comment

SebastianAment Jul 6, 2023

Choose a reason for hiding this comment

jduerholt Jul 6, 2023

Choose a reason for hiding this comment

SebastianAment Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

jduerholt Jul 6, 2023

Choose a reason for hiding this comment

jduerholt Jul 6, 2023

Choose a reason for hiding this comment

SebastianAment Jul 6, 2023

Choose a reason for hiding this comment

facebook-github-bot commented Jul 6, 2023

facebook-github-bot commented Jul 6, 2023

facebook-github-bot commented Jul 6, 2023

Enabling candidate generation with both `non_linear_constraints` and `fixed_features` #1912

Enabling candidate generation with both `non_linear_constraints` and `fixed_features` #1912

jduerholt commented Jul 3, 2023 •

edited

Loading

codecov bot commented Jul 3, 2023 •

edited

Loading

Balandat Jul 3, 2023 •

edited

Loading

SebastianAment Jul 6, 2023 •

edited

Loading