mechmotum / cyipopt

Cython interface for the interior point optimzer IPOPT
Eclipse Public License 2.0
227 stars 54 forks source link

minimize_ipopt raises TypeError for `tol` argument #235

Closed timmens closed 9 months ago

timmens commented 10 months ago

Problem

Minimization using cyipopt.minimize_ipopt raises a TypeError for the tol argument on the same code in version 1.3.0 but not in version 1.2.0. Please take a look at the minimal reproducible example below.

Minimal reproducible example

import cyipopt
import numpy as np

def fun(x):
    return np.sum(x ** 2)

res = cyipopt.minimize_ipopt(
    fun=fun,
    x0=np.zeros(2),
    tol=1e-9,
)

Error excerpt:

TypeError: Invalid option for IPOPT: b'tol': 1e-09 (Original message: "Invalid option type")

Probably caused by

It seems that cyipopt.minimize_ipopt calls the function cyipopt.scipy_interface._minimize_ipopt_iv internally (l.534), which converts the argument tol to a numpy.float (l.668), which then causes nlp.add_option(option, value) (l.598) to raise the TypeError.


Full error message

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
File ~/miniforge3/envs/[...]/lib/python3.10/site-packages/cyipopt/scipy_interface.py:599, in minimize_ipopt(fun, x0, args, kwargs, method, jac, hess, hessp, bounds, constraints, tol, callback, options)
    598 try:
--> 599     nlp.add_option(option, value)
    600 except TypeError as e:

File ~/miniforge3/envs/[...]/lib/python3.10/site-packages/cyipopt/cython/ipopt_wrapper.pyx:503, in ipopt_wrapper.Problem.add_option()

TypeError: Invalid option type

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
Cell In[2], line 12
      7     return np.sum(x ** 2)
      9 x0 = np.zeros(2)
---> 12 res = minimize_ipopt(
     13     fun=fun,
     14     x0=x0,
     15     tol=1e-9,
     16 )

File ~/miniforge3/envs/[...]/lib/python3.10/site-packages/cyipopt/scipy_interface.py:602, in minimize_ipopt(fun, x0, args, kwargs, method, jac, hess, hessp, bounds, constraints, tol, callback, options)
    600     except TypeError as e:
    601         msg = 'Invalid option for IPOPT: {0}: {1} (Original message: "{2}")'
--> 602         raise TypeError(msg.format(option, value, e))
    604 x, info = nlp.solve(x0)
    606 return OptimizeResult(x=x,
    607                       success=info['status'] == 0,
    608                       status=info['status'],
   (...)
    613                       njev=problem.njev,
    614                       nit=problem.nit)

TypeError: Invalid option for IPOPT: b'tol': 1e-09 (Original message: "Invalid option type")
moorepants commented 10 months ago

@mdhaber this may be from some changes you've added in 1.3.0. Looks like tol is converted to a numpy array.

    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.')
moorepants commented 10 months ago

I've made a simple fix for this and can make a bug fix release soon.

mdhaber commented 10 months ago

Hmm. If tol is a Python float, tol = np.asarray(tol)[()] makes it NumPy float, which is a Python float.

import numpy as np
x = 1.0
y = np.asarray(x)[()]
isinstance(y, type(x))  # True

So I didn't think that was backward incompatible. I added a test of tol for SciPy's methods to make sure it was being passed to SciPy correctly (https://github.com/mechmotum/cyipopt/blob/0b6e7494a1cafc19f0dd3b683ace7bad89e28b3c/cyipopt/tests/unit/test_scipy_optional.py#L344), but I guess there were no tests of the tol parameter before that checked it for IPOPT.

In any case, sorry for the trouble.

moorepants commented 10 months ago

No trouble, its part of the process :)