[Bug]: `gen_candidates_torch` crashes when `lr` is supplied in options

[tautomer]

What happened?

The gen_candidates_torch function in botorch/generation/gen.py has the following lines

    _optimizer = optimizer(params=[clamped_candidates], lr=options.get("lr", 0.025))

    i = 0
    stop = False
    stopping_criterion = ExpMAStoppingCriterion(**options)

The learning rate lr is an optional key in the dictionary options, but passing a dictionary with lr will apparently cause ExpMAStoppingCriterion to crash with

TypeError: ExpMAStoppingCriterion.__init__() got an unexpected keyword argument 'lr'

This happens because the safeguard _filter_kwargs was removed in this commit 04d8f05

Additionally, the current unit tests do not test the behavior of passing lr at all.

Please provide a minimal, reproducible example of the unexpected behavior.

import warnings

import torch
from botorch.acquisition import qLogExpectedImprovement
from botorch.generation.gen import gen_candidates_torch
from botorch.models import SingleTaskGP
from botorch.optim import gen_batch_initial_conditions

warnings.filterwarnings("ignore")

train_X = torch.rand(5, 2)
train_Y = torch.randn((5, 1))
model = SingleTaskGP(train_X, train_Y)
qEI = qLogExpectedImprovement(model=model, best_f=0.2)
bounds = torch.tensor([[0.0, 0.0], [1.0, 2.0]])
Xinit = gen_batch_initial_conditions(
    acq_function=qEI, bounds=bounds, q=3, num_restarts=5, raw_samples=50
)

# this works
batch_candidates, batch_acq_values = gen_candidates_torch(
    initial_conditions=Xinit,
    acquisition_function=qEI,
    lower_bounds=bounds[0],
    upper_bounds=bounds[1],
    options={"maxiter": 100},
)

# this crashes, because `ExpMAStoppingCriterion` does not have an argument of `lr`
# however, to set `lr` for the optimizer, it has to be passed in the options
batch_candidates, batch_acq_values = gen_candidates_torch(
    initial_conditions=Xinit,
    acquisition_function=qEI,
    lower_bounds=bounds[0],
    upper_bounds=bounds[1],
    options={"lr": 0.01},
)

Please paste any relevant traceback/logs produced by the example provided.

The log is from v0.11.3, but the latest v0.15.1 is the same.

botorch/generation/gen.py:396, in gen_candidates_torch(initial_conditions, acquisition_function, lower_bounds, upper_bounds, optimizer, options, callback, fixed_features, timeout_sec)
    [394](https://file+.vscode-resource.vscode-cdn.net/Users/lixinya4/codes/acquisition_functions/benchmarks/~/miniforge3/envs/obsidian/lib/python3.10/site-packages/botorch/generation/gen.py:394) i = 0
    [395](https://file+.vscode-resource.vscode-cdn.net/Users/lixinya4/codes/acquisition_functions/benchmarks/~/miniforge3/envs/obsidian/lib/python3.10/site-packages/botorch/generation/gen.py:395) stop = False
--> [396](https://file+.vscode-resource.vscode-cdn.net/Users/lixinya4/codes/acquisition_functions/benchmarks/~/miniforge3/envs/obsidian/lib/python3.10/site-packages/botorch/generation/gen.py:396) stopping_criterion = ExpMAStoppingCriterion(**options)
    [397](https://file+.vscode-resource.vscode-cdn.net/Users/lixinya4/codes/acquisition_functions/benchmarks/~/miniforge3/envs/obsidian/lib/python3.10/site-packages/botorch/generation/gen.py:397) while not stop:
    [398](https://file+.vscode-resource.vscode-cdn.net/Users/lixinya4/codes/acquisition_functions/benchmarks/~/miniforge3/envs/obsidian/lib/python3.10/site-packages/botorch/generation/gen.py:398)     i += 1

TypeError: ExpMAStoppingCriterion.__init__() got an unexpected keyword argument 'lr'

BoTorch Version

0.11.3, 0.15.1

Python Version

No response

Operating System

No response

(Optional) Describe any potential fixes you've considered to the issue outlined above.

This can be fixed by

• Re-introducing _filter_kwargs here (we probably do not want to bring it back)
• Separating lr and options
• Handling lr explicitly in the code

Personally, I would go with option 3, as we know exactly what we are expecting.

Here are all possible arguments for options

• lr
• maxiter
• minimize
• n_window
• eta
• rel_tol

Other than lr, all the rest should go to ExpMAStoppingCriterion. So we can do

    lr = options.pop("lr", 0.025)
    _optimizer = optimizer(params=[clamped_candidates], lr=lr)

Then pass the rest to ExpMAStoppingCriterion

Unless a new early stopping or optimizer is added, this should be safe. All other keys should result in an error.

        options: Options used to control the optimization. Includes
            maxiter: Maximum number of iterations

The docstring could add some details as well.

I am happy to submit a PR to fix this following this pop way if you would like.

Pull Request

Yes

Code of Conduct

• I agree to follow BoTorch's Code of Conduct

[Balandat]

Thanks for flagging this. Looks like currently the only option we allow to configure for the Optimizer is the lr argument. This seems unnecessarily restrictive; the current setup of having the options dict be shared between the optimizer and the stopping criterion (here only ExpMAStoppingCriterion since it is hard-coded) is also weird.

What do you think about the following? Instead of dealing with the learning rate, we add an optimizer_options dict to the signature of gen_candidates_torch so these are separate from the options used for the stopping criterion
Alternatively, we could also have optimizer_options be a key in the options dict, then we can pass all the specified options to the optimizer.