Remove _filter_kwargs (#2341)

Summary:
Pull Request resolved: #2341

Now when incorrect arguments are passed to `optimize_acqf` in `gen_kwargs`, they will raise an exception (as incorrect arguments generally do) rather than being silently ignored.

There was only one usage of `_filter_kwargs`, and it would not be triggered by correct usage after a prior change that stopped creating unused arguments.

Reviewed By: Balandat

Differential Revision: D57250387

fbshipit-source-id: b2090154151d88a7a34b978f1ca067222b44da59

Author: esantorella

botorch/optim/optimize.py

@@ -36,7 +36,6 @@
     TGenInitialConditions,
 )
 from botorch.optim.stopping import ExpMAStoppingCriterion
-from botorch.optim.utils import _filter_kwargs
 from torch import Tensor
 
 INIT_OPTION_KEYS = {
@@ -314,8 +313,6 @@ def _optimize_batch_candidates() -> Tuple[Tensor, Tensor, List[Warning]]:
             "timeout_sec": timeout_sec,
         }
 
-        # only add parameter constraints to gen_kwargs if they are specified
-        # to avoid unnecessary warnings in _filter_kwargs
         for constraint_name in [
             "inequality_constraints",
             "equality_constraints",
@@ -324,8 +321,6 @@ def _optimize_batch_candidates() -> Tuple[Tensor, Tensor, List[Warning]]:
             if (constraint := getattr(opt_inputs, constraint_name)) is not None:
                 gen_kwargs[constraint_name] = constraint
 
-        filtered_gen_kwargs = _filter_kwargs(opt_inputs.gen_candidates, **gen_kwargs)
-
         for i, batched_ics_ in enumerate(batched_ics):
             # optimize using random restart optimization
             with warnings.catch_warnings(record=True) as ws:
@@ -334,7 +329,7 @@ def _optimize_batch_candidates() -> Tuple[Tensor, Tensor, List[Warning]]:
                     batch_candidates_curr,
                     batch_acq_values_curr,
                 ) = opt_inputs.gen_candidates(
-                    batched_ics_, opt_inputs.acq_function, **filtered_gen_kwargs
+                    batched_ics_, opt_inputs.acq_function, **gen_kwargs
                 )
             opt_warnings += ws
             batch_candidates_list.append(batch_candidates_curr)

botorch/optim/utils/__init__.py

@@ -10,7 +10,6 @@
     get_X_baseline,
 )
 from botorch.optim.utils.common import (
-    _filter_kwargs,
     _handle_numerical_errors,
     _warning_handler_template,
 )
@@ -31,7 +30,6 @@
 from botorch.optim.utils.timeout import minimize_with_timeout
 
 __all__ = [
-    "_filter_kwargs",
     "_handle_numerical_errors",
     "_warning_handler_template",
     "as_ndarray",

botorch/optim/utils/common.py

@@ -8,34 +8,14 @@
 
 from __future__ import annotations
 
-from inspect import signature
 from logging import debug as logging_debug
-from typing import Any, Callable, Optional, Tuple
-from warnings import warn, warn_explicit, WarningMessage
+from typing import Callable, Optional, Tuple
+from warnings import warn_explicit, WarningMessage
 
 import numpy as np
 from linear_operator.utils.errors import NanError, NotPSDError
 
 
-def _filter_kwargs(function: Callable, **kwargs: Any) -> Any:
-    r"""Filter out kwargs that are not applicable for a given function.
-    Return a copy of given kwargs dict with only the required kwargs."""
-    allowed_params = signature(function).parameters
-    removed = {k for k in kwargs.keys() if k not in allowed_params}
-    if len(removed) > 0:
-        fn_descriptor = (
-            f" for function {function.__name__}"
-            if hasattr(function, "__name__")
-            else ""
-        )
-        warn(
-            f"Keyword arguments {list(removed)} will be ignored because they are"
-            f" not allowed parameters{fn_descriptor}. Allowed "
-            f"parameters are {list(allowed_params.keys())}."
-        )
-    return {k: v for k, v in kwargs.items() if k not in removed}
-
-
 def _handle_numerical_errors(
     error: RuntimeError, x: np.ndarray, dtype: Optional[np.dtype] = None
 ) -> Tuple[np.ndarray, np.ndarray]:

test/acquisition/test_knowledge_gradient.py

@@ -24,10 +24,8 @@
 )
 from botorch.acquisition.utils import project_to_sample_points
 from botorch.exceptions.errors import UnsupportedError
-from botorch.generation.gen import gen_candidates_scipy
 from botorch.models import SingleTaskGP
 from botorch.optim.optimize import optimize_acqf
-from botorch.optim.utils import _filter_kwargs
 from botorch.posteriors.gpytorch import GPyTorchPosterior
 from botorch.sampling.normal import IIDNormalSampler, SobolQMCNormalSampler
 from botorch.utils.test_helpers import DummyNonScalarizingPosteriorTransform
@@ -588,11 +586,6 @@ def test_optimize_w_posterior_transform(self):
                         torch.zeros(2, n_f + 1, 2, **tkwargs),
                         torch.zeros(2, **tkwargs),
                     ),
-                ), mock.patch(
-                    f"{optimize_acqf.__module__}._filter_kwargs",
-                    wraps=lambda f, **kwargs: _filter_kwargs(
-                        function=gen_candidates_scipy, **kwargs
-                    ),
                 ):
 
                     candidate, value = optimize_acqf(

test/optim/test_optimize.py

@@ -6,7 +6,6 @@
 
 import itertools
 import warnings
-from inspect import signature
 from itertools import product
 from unittest import mock
 
@@ -114,10 +113,8 @@ class TestOptimizeAcqf(BotorchTestCase):
     @mock.patch("botorch.generation.gen.gen_candidates_torch")
     @mock.patch("botorch.optim.optimize.gen_batch_initial_conditions")
     @mock.patch("botorch.optim.optimize.gen_candidates_scipy")
-    @mock.patch("botorch.optim.utils.common.signature")
     def test_optimize_acqf_joint(
         self,
-        mock_signature,
         mock_gen_candidates_scipy,
         mock_gen_batch_initial_conditions,
         mock_gen_candidates_torch,
@@ -134,10 +131,6 @@ def test_optimize_acqf_joint(
                 mock_gen_candidates_scipy,
                 mock_gen_candidates_torch,
             ):
-                if mock_gen_candidates == mock_gen_candidates_torch:
-                    mock_signature.return_value = signature(gen_candidates_torch)
-                else:
-                    mock_signature.return_value = signature(gen_candidates_scipy)
 
                 mock_gen_batch_initial_conditions.return_value = torch.zeros(
                     num_restarts, q, 3, device=self.device, dtype=dtype
@@ -264,12 +257,14 @@ def test_optimize_acqf_joint(
             )
 
     @mock.patch("botorch.optim.optimize.gen_batch_initial_conditions")
-    @mock.patch("botorch.optim.optimize.gen_candidates_scipy")
-    @mock.patch("botorch.generation.gen.gen_candidates_torch")
-    @mock.patch("botorch.optim.utils.common.signature")
+    @mock.patch(
+        "botorch.optim.optimize.gen_candidates_scipy", wraps=gen_candidates_scipy
+    )
+    @mock.patch(
+        "botorch.generation.gen.gen_candidates_torch", wraps=gen_candidates_torch
+    )
     def test_optimize_acqf_sequential(
         self,
-        mock_signature,
         mock_gen_candidates_torch,
         mock_gen_candidates_scipy,
         mock_gen_batch_initial_conditions,
@@ -278,11 +273,6 @@ def test_optimize_acqf_sequential(
         for mock_gen_candidates, timeout_sec in product(
             [mock_gen_candidates_scipy, mock_gen_candidates_torch], [None, 1e-4]
         ):
-            if mock_gen_candidates == mock_gen_candidates_torch:
-                mock_signature.return_value = signature(gen_candidates_torch)
-            else:
-                mock_signature.return_value = signature(gen_candidates_scipy)
-            mock_gen_candidates.__name__ = "gen_candidates"
             q = 3
             num_restarts = 2
             raw_samples = 10
@@ -1019,16 +1009,12 @@ def nlc4(x):
                     raw_samples=16,
                 )
 
-    @mock.patch("botorch.generation.gen.gen_candidates_torch")
     @mock.patch("botorch.optim.optimize.gen_batch_initial_conditions")
     @mock.patch("botorch.optim.optimize.gen_candidates_scipy")
-    @mock.patch("botorch.optim.utils.common.signature")
     def test_optimize_acqf_non_linear_constraints_sequential(
         self,
-        mock_signature,
         mock_gen_candidates_scipy,
         mock_gen_batch_initial_conditions,
-        mock_gen_candidates_torch,
     ):
         def nlc(x):
             return 4 * x[..., 2] - 5
@@ -1037,90 +1023,63 @@ def nlc(x):
         num_restarts = 2
         raw_samples = 10
         options = {}
-        for mock_gen_candidates in (
-            mock_gen_candidates_torch,
-            mock_gen_candidates_scipy,
-        ):
-            if mock_gen_candidates == mock_gen_candidates_torch:
-                mock_signature.return_value = signature(gen_candidates_torch)
-            else:
-                mock_signature.return_value = signature(gen_candidates_scipy)
-            for dtype in (torch.float, torch.double):
-                mock_acq_function = MockAcquisitionFunction()
-                mock_gen_batch_initial_conditions.side_effect = [
-                    torch.zeros(num_restarts, 1, 3, device=self.device, dtype=dtype)
-                    for _ in range(q)
-                ]
-                gcs_return_vals = [
-                    (
-                        torch.tensor(
-                            [[[1.0, 2.0, 3.0]]], device=self.device, dtype=dtype
-                        ),
-                        torch.tensor([i], device=self.device, dtype=dtype),
-                    )
-                    # for nonlinear inequality constraints the batch_limit variable is
-                    # currently set to 1 by default and hence gen_candidates_scipy is
-                    # called num_restarts*q times
-                    for i in range(num_restarts * q)
-                ]
-                mock_gen_candidates.side_effect = gcs_return_vals
-                expected_candidates = torch.cat(
-                    [cands[0] for cands, _ in gcs_return_vals[::num_restarts]], dim=-2
+
+        for dtype in (torch.float, torch.double):
+            mock_acq_function = MockAcquisitionFunction()
+            mock_gen_batch_initial_conditions.side_effect = [
+                torch.zeros(num_restarts, 1, 3, device=self.device, dtype=dtype)
+                for _ in range(q)
+            ]
+            gcs_return_vals = [
+                (
+                    torch.tensor([[[1.0, 2.0, 3.0]]], device=self.device, dtype=dtype),
+                    torch.tensor([i], device=self.device, dtype=dtype),
                 )
-                bounds = torch.stack(
-                    [
-                        torch.zeros(3, device=self.device, dtype=dtype),
-                        4 * torch.ones(3, device=self.device, dtype=dtype),
-                    ]
+                # for nonlinear inequality constraints the batch_limit variable is
+                # currently set to 1 by default and hence gen_candidates_scipy is
+                # called num_restarts*q times
+                for i in range(num_restarts * q)
+            ]
+            mock_gen_candidates_scipy.side_effect = gcs_return_vals
+            expected_candidates = torch.cat(
+                [cands[0] for cands, _ in gcs_return_vals[::num_restarts]], dim=-2
+            )
+            bounds = torch.stack(
+                [
+                    torch.zeros(3, device=self.device, dtype=dtype),
+                    4 * torch.ones(3, device=self.device, dtype=dtype),
+                ]
+            )
+            with warnings.catch_warnings(record=True) as ws:
+                candidates, acq_value = optimize_acqf(
+                    acq_function=mock_acq_function,
+                    bounds=bounds,
+                    q=q,
+                    num_restarts=num_restarts,
+                    raw_samples=raw_samples,
+                    options=options,
+                    nonlinear_inequality_constraints=[nlc],
+                    sequential=True,
+                    ic_generator=mock_gen_batch_initial_conditions,
+                    gen_candidates=mock_gen_candidates_scipy,
                 )
-                with warnings.catch_warnings(record=True) as ws:
-                    candidates, acq_value = optimize_acqf(
-                        acq_function=mock_acq_function,
-                        bounds=bounds,
-                        q=q,
-                        num_restarts=num_restarts,
-                        raw_samples=raw_samples,
-                        options=options,
-                        nonlinear_inequality_constraints=[nlc],
-                        sequential=True,
-                        ic_generator=mock_gen_batch_initial_conditions,
-                        gen_candidates=mock_gen_candidates,
-                    )
-                if mock_gen_candidates == mock_gen_candidates_torch:
-                    self.assertEqual(len(ws), 3)
-                    message = (
-                        "Keyword arguments ['nonlinear_inequality_constraints']"
-                        " will be ignored because they are not allowed parameters for"
-                        " function gen_candidates. Allowed parameters are "
-                        " ['initial_conditions', 'acquisition_function', "
-                        "'lower_bounds', 'upper_bounds', 'optimizer', 'options',"
-                        " 'callback', 'fixed_features', 'timeout_sec']."
-                    )
-                    expected_warning_raised = (
-                        issubclass(w.category, UserWarning)
-                        and message == str(w.message)
-                        for w in ws
-                    )
-                    self.assertTrue(expected_warning_raised)
-                    # check message
-                else:
-                    self.assertEqual(len(ws), 0)
-                self.assertTrue(torch.equal(candidates, expected_candidates))
-                # Extract the relevant entries from gcs_return_vals to
-                # perform comparison with.
-                self.assertTrue(
-                    torch.equal(
-                        acq_value,
-                        torch.cat(
-                            [
-                                expected_acq_value
-                                for _, expected_acq_value in gcs_return_vals[
-                                    num_restarts - 1 :: num_restarts
-                                ]
+                self.assertEqual(len(ws), 0)
+            self.assertTrue(torch.equal(candidates, expected_candidates))
+            # Extract the relevant entries from gcs_return_vals to
+            # perform comparison with.
+            self.assertTrue(
+                torch.equal(
+                    acq_value,
+                    torch.cat(
+                        [
+                            expected_acq_value
+                            for _, expected_acq_value in gcs_return_vals[
+                                num_restarts - 1 :: num_restarts
                             ]
-                        ),
+                        ]
                     ),
-                )
+                ),
+            )
 
     def test_constraint_caching(self):
         def nlc(x):

test/optim/utils/test_common.py

@@ -10,40 +10,12 @@
 from warnings import catch_warnings, warn
 
 import numpy as np
-from botorch.optim.utils import (
-    _filter_kwargs,
-    _handle_numerical_errors,
-    _warning_handler_template,
-)
+from botorch.optim.utils import _handle_numerical_errors, _warning_handler_template
 from botorch.utils.testing import BotorchTestCase
 from linear_operator.utils.errors import NanError, NotPSDError
 
 
 class TestUtilsCommon(BotorchTestCase):
-    def test__filter_kwargs(self) -> None:
-        def mock_adam(params, lr: float = 0.001) -> None:
-            return  # pragma: nocover
-
-        kwargs = {"lr": 0.01, "maxiter": 3000}
-        expected_msg = (
-            r"Keyword arguments \['maxiter'\] will be ignored because they are "
-            r"not allowed parameters for function mock_adam. Allowed parameters "
-            r"are \['params', 'lr'\]."
-        )
-
-        with self.assertWarnsRegex(Warning, expected_msg):
-            valid_kwargs = _filter_kwargs(mock_adam, **kwargs)
-        self.assertEqual(set(valid_kwargs.keys()), {"lr"})
-
-        mock_partial = partial(mock_adam, lr=2.0)
-        expected_msg = (
-            r"Keyword arguments \['maxiter'\] will be ignored because they are "
-            r"not allowed parameters. Allowed parameters are \['params', 'lr'\]."
-        )
-        with self.assertWarnsRegex(Warning, expected_msg):
-            valid_kwargs = _filter_kwargs(mock_partial, **kwargs)
-        self.assertEqual(set(valid_kwargs.keys()), {"lr"})
-
     def test_handle_numerical_errors(self):
         x = np.zeros(1, dtype=np.float64)