botorch icon indicating copy to clipboard operation
botorch copied to clipboard

[Bug] [relatively easy fix] `gen_one_shot_kg_initial_conditions` should error when provided inter-point constraints

Open esantorella opened this issue 2 years ago • 1 comments

🐛 Bug

See https://github.com/pytorch/botorch/issues/1220 for context. optimize_acqf does not support inter-point constraints -- those whose indices are two-dimensional -- when the acquisition function is qKnowledgeGradient. This is because gen_one_shot_kg_initial_conditions does not support inter-point constraints. Since typing constraints correctly is confusing, BoTorch should raise an informative error rather than leaving the user to get a confusing IndexError and wonder if they typed the constraints wrong :)

To reproduce

from botorch.acquisition import qKnowledgeGradient
from botorch.fit import fit_gpytorch_mll
from botorch.models import SingleTaskGP
from gpytorch.mlls import ExactMarginalLogLikelihood

x = torch.rand((20, 2))
y = x.sum(1, keepdim=True) - 1
model = SingleTaskGP(train_X=x, train_Y=y)
mll = ExactMarginalLogLikelihood(model.likelihood, model)
 _ = fit_gpytorch_mll(mll)

bounds = torch.tensor([[0, 0], [1, 1]]).to(x)
q = 2
# should have
# samples[:, 0, 0] * 1 + samples[:, 1, 1] >= 0.5
# where samples is n x q x d
indices = torch.tensor([[0, 0], [1, 1]])
inequality_constraints = [(indices, torch.ones(2).to(x), 0.5)]

acqf = qKnowledgeGradient(model, num_fantasies=8)
candidates, acq_vals = optimize_acqf(
        acq_function=acqf,
        bounds=bounds,
        q=q,
        num_restarts=10,
        raw_samples=64,
        inequality_constraints=inequality_constraints,
)

** Stack trace/error message **

 1) IndexError: index 3 is out of bounds for dimension 0 with size 2
      File "pytorch/botorch/test/optim/test_optimize.py", line 265, in ftest_optimize_acqf_kg_constrained
        candidates, acq_vals = optimize_acqf(
      File "botorch/optim/optimize.py", line 557, in optimize_acqf
        return _optimize_acqf(opt_acqf_inputs)
      File "botorch/optim/optimize.py", line 586, in _optimize_acqf
        return _optimize_acqf_batch(
      File "botorch/optim/optimize.py", line 269, in _optimize_acqf_batch
        batch_initial_conditions = opt_inputs.get_ic_generator()(
      File "botorch/optim/initializers.py", line 570, in gen_one_shot_kg_initial_conditions
        fantasy_cands, fantasy_vals = optimize_acqf(
      File "botorch/optim/optimize.py", line 557, in optimize_acqf
        return _optimize_acqf(opt_acqf_inputs)
      File "botorch/optim/optimize.py", line 586, in _optimize_acqf
        return _optimize_acqf_batch(
      File "botorch/optim/optimize.py", line 269, in _optimize_acqf_batch
        batch_initial_conditions = opt_inputs.get_ic_generator()(
      File "botorch/optim/initializers.py", line 415, in gen_batch_initial_conditions
        X_rnd = sample_q_batches_from_polytope(
      File "botorch/optim/initializers.py", line 272, in sample_q_batches_from_polytope
        samples = get_polytope_samples(
      File "botorch/utils/sampling.py", line 800, in get_polytope_samples
        constraints = normalize_linear_constraints(bounds, inequality_constraints)
      File "botorch/utils/sampling.py", line 754, in normalize_linear_constraints
        lower, upper = bounds[:, index]

Expected Behavior

An informative error should be raised explaining that this is not supported. I think the fix here is that gen_one_shot_kg_initial_conditions should raise an UnsupportedError or NotImplementedError if provided inter-point constraints. However, I'm not 100% sure that gen_one_shot_kg_initial_conditions is the right place for the error, so whoever fixes this should carefully verify that the error is accurate and that usages that don't trigger the error do work.

esantorella avatar May 31 '23 23:05 esantorella

I have been messing with this recently to enable this for a certain kind of budget constraints where we can generate the one-shot initial conditions by doing some sampling from the simplex. It's a bad hack right now, but I'll try to figure out how to make this more reasonable, in which case we should be able to support this.

Balandat avatar Jun 01 '23 00:06 Balandat

Hi @esantorella , @Balandat !

According to me , I would put an explicit check in gen_one_shot_kg_initial_conditions to check for inter-point inequality constraints by simply checking if the constraint indices are over multiple points in the q-batch. If that's the case, I would raise an obvious UnsupportedError and an explicit message to clearly signal to the user that inter-point inequality constraints are unsupported and reduce any ambiguity associated with index errors.

Could you please assign this issue to me ? So that I'd work on it .

Thank You !

Muhammad-Rebaal avatar May 11 '25 16:05 Muhammad-Rebaal

@Muhammad-Rebaal sure thing, definitely a good idea to raise an informative error for now. Thanks for working on this!

Balandat avatar May 11 '25 19:05 Balandat

Actually, @Muhammad-Rebaal, looks like this was already done recently in #2775. Let me close out this issue.

Balandat avatar May 11 '25 19:05 Balandat

Actually, @Muhammad-Rebaal, looks like this was already done recently in #2775. Let me close out this issue.

Ok , Is there any community channel for the contributors ?

Muhammad-Rebaal avatar May 11 '25 20:05 Muhammad-Rebaal

We don't have a dedicated community channel at this point. In the past we have coordinated with contributors on the pytorch slack for specific (and more complex) issues, but we try to keep higher level conversation on github as much as possible.

Balandat avatar May 11 '25 20:05 Balandat

We don't have a dedicated community channel at this point. In the past we have coordinated with contributors on the pytorch slack for specific (and more complex) issues, but we try to keep higher level conversation on github as much as possible.

Ok sure ! In this case , I'll keep the conversation on GitHub , to solve the issues .

Thanks for a quick response!

Muhammad-Rebaal avatar May 11 '25 20:05 Muhammad-Rebaal