Skip to content

Commit de1eb9b

Browse files
saitcakmakmeta-codesync[bot]
authored andcommitted
Fix double-application of fixed_features to nonlinear constraints (#3332) (#3333)
Summary: Fixes #3332. When both `fixed_features` and `nonlinear_inequality_constraints` are passed to `optimize_acqf` / `gen_candidates_scipy`, the fixed features were being applied to the nonlinear constraints **twice**, scrambling the constraint's input and causing spurious SLSQP failures and "infeasible candidate" errors. ## Root cause In `gen_candidates_scipy`, the nonlinear constraints go through two independent fixed-feature transforms: 1. `_remove_fixed_features_from_optimization` → `_generate_unfixed_nonlin_constraints` wraps each constraint so it accepts the **reduced** (unfixed) variable and re-inserts the fixed features internally (via `cat` + a `selector` permutation). 2. The wrapper passed to `make_scipy_nonlinear_inequality_constraints` was `f_np_wrapper_`, which carries `fixed_features=fixed_features_` and **also** re-inserts the fixed feature (via `fix_features`) before evaluating the constraint. So the fixed value gets inserted twice. For `d=5`, `fixed_features={0: 1.0}` (`selector=[4,0,1,2,3]`), the reduced `[x1,x2,x3,x4]` first becomes `[1.0,x1,x2,x3,x4]` (from `fix_features`), then the constraint wrapper appends `1.0` and applies `selector`, yielding `[x4, 1.0, x1, x2, x3]` — exactly the scramble reported in #3332. The acquisition objective wrapper (`f_np_wrapper_`) is correct as-is, since the acquisition function genuinely lives in the full space; only the already-transformed constraints were over-corrected. ## Fix Use a separate wrapper with `fixed_features=None` for the already-transformed nonlinear constraints, as suggested in the issue. ## Testing - The existing fixed-features + nonlinear test (`test_optimize_acqf_nonlinear_constraints`) used a permutation-invariant constraint (`4 - x.sum()`) with a `torch.sort()` assertion, so it could not detect coordinate scrambling. - Added `test_optimize_acqf_nonlinear_constraints_with_fixed_features` with a **coordinate-sensitive** constraint and an order-sensitive assertion. Mutation-checked: it fails on the unpatched code and passes with the fix. - `test_optimize.py`, `test_gen.py`, `test_parameter_constraints.py`, `test_utils.py` all pass. Pull Request resolved: #3333 Reviewed By: jandylin Differential Revision: D109710542 Pulled By: saitcakmak fbshipit-source-id: bf60b07cfe81b033ab04fd8afb913475f16bf0fe
1 parent ade1785 commit de1eb9b

2 files changed

Lines changed: 68 additions & 1 deletion

File tree

botorch/generation/gen.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,9 +358,19 @@ def f(x):
358358
nl_ineq_constraints = (
359359
_no_fixed_features.nonlinear_inequality_constraints
360360
)
361+
# The constraints in ``nl_ineq_constraints`` have already been
362+
# transformed by ``_generate_unfixed_nonlin_constraints`` to operate
363+
# on the reduced (unfixed) variable and re-insert the fixed features
364+
# internally. We therefore must NOT re-apply ``fixed_features`` in the
365+
# wrapper, otherwise the fixed features would be inserted twice,
366+
# producing a wrongly permuted input to the constraint (see #3332).
367+
constraint_f_np_wrapper = partial(
368+
f_np_wrapper,
369+
fixed_features=None,
370+
)
361371
constraints += make_scipy_nonlinear_inequality_constraints(
362372
nonlinear_inequality_constraints=nl_ineq_constraints,
363-
f_np_wrapper=f_np_wrapper_,
373+
f_np_wrapper=constraint_f_np_wrapper,
364374
x0=x0,
365375
shapeX=candidates_.shape,
366376
)

test/optim/test_optimize.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,17 @@ def forward(self, X):
9494
return torch.linalg.norm(X, dim=-1).squeeze(-1)
9595

9696

97+
class NegSquaredDistanceAcquisitionFunction(AcquisitionFunction):
98+
"""Negative squared distance to a target, summed over the q-batch."""
99+
100+
def __init__(self, target: Tensor, model=None): # noqa: D107
101+
super().__init__(model=model)
102+
self.register_buffer("target", target)
103+
104+
def forward(self, X):
105+
return -((X - self.target.to(X)) ** 2).sum(dim=-1).sum(dim=-1)
106+
107+
97108
class MockOneShotEvaluateAcquisitionFunction(MockOneShotAcquisitionFunction):
98109
def evaluate(self, X: Tensor, bounds: Tensor):
99110
return X.sum()
@@ -1337,6 +1348,52 @@ def nlc4(x):
13371348
raw_samples=16,
13381349
)
13391350

1351+
def test_optimize_acqf_nonlinear_constraints_with_fixed_features(self):
1352+
# Regression test for
1353+
# https://github.com/meta-pytorch/botorch/issues/3332
1354+
# When ``fixed_features`` and ``nonlinear_inequality_constraints`` are
1355+
# combined, the constraints are transformed by
1356+
# ``_generate_unfixed_nonlin_constraints`` to operate on the reduced
1357+
# (unfixed) variable and re-insert the fixed features internally. The
1358+
# SLSQP wrapper must therefore NOT re-apply ``fixed_features`` on top,
1359+
# otherwise the fixed features are inserted twice and the constraint
1360+
# receives a wrongly permuted input.
1361+
for dtype in (torch.float, torch.double):
1362+
tkwargs = {"device": self.device, "dtype": dtype}
1363+
target = torch.tensor([1.0, 0.05, 0.5, 0.5, 0.5], **tkwargs)
1364+
acqf = NegSquaredDistanceAcquisitionFunction(target=target)
1365+
bounds = torch.tensor(
1366+
[[0.0, 0.0, 0.0, 0.0, 0.0], [5.0, 1.0, 1.0, 1.0, 1.0]], **tkwargs
1367+
)
1368+
1369+
# Coordinate-sensitive constraint: x2 * x3 / 2 - x1 >= 0. This is not
1370+
# permutation invariant, so a scrambled input order would change its
1371+
# value. Feasible at the target: 0.5 * 0.5 / 2 - 0.05 = 0.075 >= 0.
1372+
def constraint(x):
1373+
return x[..., 2] * x[..., 3] / 2.0 - x[..., 1]
1374+
1375+
ic = torch.tensor([[[1.0, 0.20, 0.70, 0.70, 0.80]]], **tkwargs)
1376+
# The initial condition is feasible for the original full constraint.
1377+
self.assertGreaterEqual(constraint(ic).item(), 0.0)
1378+
1379+
candidate, _ = optimize_acqf(
1380+
acq_function=acqf,
1381+
bounds=bounds,
1382+
q=1,
1383+
num_restarts=1,
1384+
raw_samples=16,
1385+
fixed_features={0: 1.0},
1386+
nonlinear_inequality_constraints=[(constraint, True)],
1387+
batch_initial_conditions=ic,
1388+
options={"method": "SLSQP", "batch_limit": 1, "maxiter": 200},
1389+
)
1390+
# The fixed feature is respected.
1391+
self.assertAlmostEqual(candidate[0, 0].item(), 1.0, places=4)
1392+
# The returned candidate satisfies the original full-dim constraint.
1393+
self.assertGreaterEqual(constraint(candidate).item(), -1e-6)
1394+
# And it converges to the (feasible) target.
1395+
self.assertAllClose(candidate, target.unsqueeze(0), atol=1e-2)
1396+
13401397
@mock.patch("botorch.optim.optimize.gen_batch_initial_conditions")
13411398
@mock.patch("botorch.optim.optimize.gen_candidates_scipy")
13421399
def test_optimize_acqf_non_linear_constraints_sequential(

0 commit comments

Comments
 (0)