From ddec62ddb0fae589e66100c04bafde7059903e82 Mon Sep 17 00:00:00 2001 From: Matt Haberland Date: Thu, 18 May 2023 18:37:01 -0700 Subject: [PATCH 1/2] MAINT: minimize_cyipopt: add input validation --- cyipopt/scipy_interface.py | 41 ++++++++++++++++++ cyipopt/tests/unit/test_scipy_optional.py | 52 +++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/cyipopt/scipy_interface.py b/cyipopt/scipy_interface.py index b51df799..1a64c1b7 100644 --- a/cyipopt/scipy_interface.py +++ b/cyipopt/scipy_interface.py @@ -465,6 +465,11 @@ def minimize_ipopt(fun, msg = 'Install SciPy to use the `minimize_ipopt` function.' raise ImportError(msg) + res = _minimize_ipopt_iv(fun, x0, args, kwargs, method, jac, hess, hessp, + bounds, constraints, tol, callback, options) + (fun, x0, args, kwargs, method, jac, hess, hessp, + bounds, constraints, tol, callback, options) = res + _x0 = np.atleast_1d(x0) lb, ub = get_bounds(bounds) @@ -533,3 +538,39 @@ def minimize_ipopt(fun, nfev=problem.nfev, njev=problem.njev, nit=problem.nit) + +def _minimize_ipopt_iv(fun, x0, args, kwargs, method, jac, hess, hessp, + bounds, constraints, tol, callback, options): + # basic input validation for minimize_ipopt + if fun is not None and not callable(fun): + raise ValueError('`fun` must be callable.') + x0 = np.asarray(x0)[()] + if not np.issubdtype(x0.dtype, np.number): + raise ValueError('`x0` must be a numeric array.') + if not np.iterable(args): + args = (args,) + kwargs = dict() if kwargs is None else kwargs + if not isinstance(kwargs, dict): + raise ValueError('`kwargs` must be a dictionary.') + if method is not None: # this will be updated when gh-200 is merged + raise NotImplementedError('`method` is not yet supported.`') + if jac is not None and jac not in {True, False} and not callable(jac): + raise ValueError('`jac` must be callable or boolean.') + if hess is not None and not callable(hess): + raise ValueError('`hess` must be callable.') + if hessp is not None: + raise NotImplementedError('`hessp` is not yet supported by Ipopt.`') + # TODO: add input validation for `bounds` and `constraints` when adding + # support for instances of new-style constraints (e.g. `Bounds` and + # `NonlinearConstraint`) and sequences of constraints. + if callback is not None: + raise NotImplementedError('`callback` is not yet supported by Ipopt.`') + if tol is not None: + tol = np.asarray(tol)[()] + if tol.ndim != 0 or not np.issubdtype(tol.dtype, np.number) or tol <= 0: + raise ValueError('`tol` must be a positive scalar.') + options = dict() if options is None else options + if not isinstance(options, dict): + raise ValueError('`options` must be a dictionary.') + return (fun, x0, args, kwargs, method, jac, hess, hessp, + bounds, constraints, tol, callback, options) diff --git a/cyipopt/tests/unit/test_scipy_optional.py b/cyipopt/tests/unit/test_scipy_optional.py index 0bdc2bc6..e636a613 100644 --- a/cyipopt/tests/unit/test_scipy_optional.py +++ b/cyipopt/tests/unit/test_scipy_optional.py @@ -25,6 +25,58 @@ def test_minimize_ipopt_import_error_if_no_scipy(): cyipopt.minimize_ipopt(None, None) +@pytest.mark.skipif("scipy" not in sys.modules, + reason="Test only valid if Scipy available.") +def test_minimize_ipopt_input_validation(): + x0 = 1 + def f(x): + return x**2 + + message = "`fun` must be callable." + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt('migratory coconuts', x0) + + message = "`x0` must be a numeric array." + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, 'spamalot') + + message = "`kwargs` must be a dictionary." + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, x0, kwargs='elderberries') + + message = "`method` is not yet supported." + with pytest.raises(NotImplementedError, match=message): + cyipopt.minimize_ipopt(f, x0, method='a newt') + + message = "`jac` must be callable or boolean." + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, x0, jac='self-perpetuating autocracy') + + message = "`hess` must be callable." + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, x0, hess='farcical aquatic ceremony') + + message = "`hessp` is not yet supported by Ipopt." + with pytest.raises(NotImplementedError, match=message): + cyipopt.minimize_ipopt(f, x0, hessp='shrubbery') + + message = "`callback` is not yet supported by Ipopt." + with pytest.raises(NotImplementedError, match=message): + cyipopt.minimize_ipopt(f, x0, callback='a duck') + + message = "`tol` must be a positive scalar." + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, x0, tol=[1, 2, 3]) + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, x0, tol='ni') + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, x0, tol=-1) + + message = "`options` must be a dictionary." + with pytest.raises(ValueError, match=message): + cyipopt.minimize_ipopt(f, x0, options='supreme executive power') + + @pytest.mark.skipif("scipy" not in sys.modules, reason="Test only valid if Scipy available.") def test_minimize_ipopt_if_scipy(): From 661bdcc3abac2583a7f7626fe6600945a4f8ffee Mon Sep 17 00:00:00 2001 From: Matt Haberland Date: Fri, 19 May 2023 08:18:50 -0700 Subject: [PATCH 2/2] MAINT: minimize_ipopt: split input validation --- cyipopt/scipy_interface.py | 52 +++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/cyipopt/scipy_interface.py b/cyipopt/scipy_interface.py index 1a64c1b7..b74ac010 100644 --- a/cyipopt/scipy_interface.py +++ b/cyipopt/scipy_interface.py @@ -107,9 +107,26 @@ def __init__(self, raise ImportError() self.obj_hess = None self.last_x = None + + # Input validation of user-provided arguments + if fun is not None and not callable(fun): + raise ValueError('`fun` must be callable.') + if not isinstance(args, tuple): + args = (args,) + kwargs = dict() if kwargs is None else kwargs + if not isinstance(kwargs, dict): + raise ValueError('`kwargs` must be a dictionary.') + if jac is not None and jac not in {True, False} and not callable(jac): + raise ValueError('`jac` must be callable or boolean.') + if hess is not None and not callable(hess): + raise ValueError('`hess` must be callable.') if hessp is not None: - msg = 'Using hessian matrix times an arbitrary vector is not yet implemented!' - raise NotImplementedError(msg) + raise NotImplementedError( + '`hessp` is not yet supported by Ipopt.`') + # TODO: add input validation for `constraints` when adding + # support for instances of new-style constraints (e.g. + # `NonlinearConstraint`) and sequences of constraints. + if hess is not None: self.obj_hess = hess if jac is None: @@ -118,8 +135,7 @@ def __init__(self, elif jac is True: fun = MemoizeJac(fun) jac = fun.derivative - elif not callable(jac): - raise NotImplementedError('jac has to be bool or a function') + self.fun = fun self.jac = jac self.args = args @@ -541,36 +557,30 @@ def minimize_ipopt(fun, def _minimize_ipopt_iv(fun, x0, args, kwargs, method, jac, hess, hessp, bounds, constraints, tol, callback, options): - # basic input validation for minimize_ipopt - if fun is not None and not callable(fun): - raise ValueError('`fun` must be callable.') + # basic input validation for minimize_ipopt that is not included in + # IpoptProblemWrapper + x0 = np.asarray(x0)[()] if not np.issubdtype(x0.dtype, np.number): raise ValueError('`x0` must be a numeric array.') - if not np.iterable(args): - args = (args,) - kwargs = dict() if kwargs is None else kwargs - if not isinstance(kwargs, dict): - raise ValueError('`kwargs` must be a dictionary.') + if method is not None: # this will be updated when gh-200 is merged raise NotImplementedError('`method` is not yet supported.`') - if jac is not None and jac not in {True, False} and not callable(jac): - raise ValueError('`jac` must be callable or boolean.') - if hess is not None and not callable(hess): - raise ValueError('`hess` must be callable.') - if hessp is not None: - raise NotImplementedError('`hessp` is not yet supported by Ipopt.`') - # TODO: add input validation for `bounds` and `constraints` when adding - # support for instances of new-style constraints (e.g. `Bounds` and - # `NonlinearConstraint`) and sequences of constraints. + + # TODO: add input validation for `bounds` when adding + # support for instances of new-style constraints (e.g. `Bounds`) + if callback is not None: raise NotImplementedError('`callback` is not yet supported by Ipopt.`') + if tol is not None: tol = np.asarray(tol)[()] if tol.ndim != 0 or not np.issubdtype(tol.dtype, np.number) or tol <= 0: raise ValueError('`tol` must be a positive scalar.') + options = dict() if options is None else options if not isinstance(options, dict): raise ValueError('`options` must be a dictionary.') + return (fun, x0, args, kwargs, method, jac, hess, hessp, bounds, constraints, tol, callback, options)