mechmotum / cyipopt

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

Arch-dependent failures of test_minimize_ipopt_jac_with_scipy_methods[cobyla] #237

Open musicinmybrain opened 10 months ago

musicinmybrain commented 10 months ago

While working on updating the python-cyipopt package in Fedora Linux to 1.3.0, I observed consistent failures of of test_minimize_ipopt_jac_with_scipy_methods[cobyla] on ppc64le and s390x.

In five scratch-builds, I observed:

The ppc64le and s390x failures all looked like this, with identical numeric values:

=================================== FAILURES ===================================
______________ test_minimize_ipopt_jac_with_scipy_methods[cobyla] ______________
method = 'cobyla'
    @pytest.mark.skipif("scipy" not in sys.modules,
                        reason="Test only valid if Scipy available.")
    @pytest.mark.parametrize('method', [None] + MINIMIZE_METHODS)
    def test_minimize_ipopt_jac_with_scipy_methods(method):
        x0 = [0] * 4
        a0, b0, c0, d0 = 1, 2, 3, 4
        atol, rtol = 5e-5, 5e-5

        def fun(x, a=0, e=0, b=0):
            assert a == a0
            assert b == b0
            fun.count += 1
            return (x[0] - a) ** 2 + (x[1] - b) ** 2 + x[2] ** 2 + x[3] ** 2

        def grad(x, a=0, e=0, b=0):
            assert a == a0
            assert b == b0
            grad.count += 1
            return [2 * (x[0] - a), 2 * (x[1] - b), 2 * x[2], 2 * x[3]]

        def hess(x, a=0, e=0, b=0):
            assert a == a0
            assert b == b0
            hess.count += 1
            return 2 * np.eye(4)

        def hessp(x, p, a=0, e=0, b=0):
            assert a == a0
            assert b == b0
            hessp.count += 1
            return 2 * np.eye(4) @ p

        def fun_constraint(x, c=0, e=0, d=0):
            assert c == c0
            assert d == d0
            fun_constraint.count += 1
            return [(x[2] - c) ** 2, (x[3] - d) ** 2]

        def grad_constraint(x, c=0, e=0, d=0):
            assert c == c0
            assert d == d0
            grad_constraint.count += 1
            return np.hstack((np.zeros((2, 2)),
                             np.diag([2 * (x[2] - c), 2 * (x[3] - d)])))

        def callback(*args, **kwargs):
            callback.count += 1

        constr = {
            "type": "eq",
            "fun": fun_constraint,
            "jac": grad_constraint,
            "args": (c0,),
            "kwargs": {'d': d0},
        }

        kwargs = {}
        jac_methods = {'cg', 'bfgs', 'newton-cg', 'l-bfgs-b', 'tnc', 'slsqp,',
                       'dogleg', 'trust-ncg', 'trust-krylov', 'trust-exact',
                       'trust-constr'}
        hess_methods = {'newton-cg', 'dogleg', 'trust-ncg', 'trust-krylov',
                       'trust-exact', 'trust-constr'}
        hessp_methods = hess_methods - {'dogleg', 'trust-exact'}
        constr_methods = {'slsqp', 'trust-constr'}

        if method in jac_methods:
            kwargs['jac'] = grad
        if method in hess_methods:
            kwargs['hess'] = hess
        if method in constr_methods:
            kwargs['constraints'] = constr
        if method in MINIMIZE_METHODS:
            kwargs['callback'] = callback

        fun.count = 0
        grad.count = 0
        hess.count = 0
        hessp.count = 0
        fun_constraint.count = 0
        grad_constraint.count = 0
        callback.count = 0

        res = cyipopt.minimize_ipopt(fun, x0, method=method, args=(a0,),
                                     kwargs={'b': b0}, **kwargs)

        assert res.success
        np.testing.assert_allclose(res.x[:2], [a0, b0], rtol=rtol)

        # confirm that the test covers what we think it does: all the functions
        # that we provide are actually being executed; that is, the assertions
        # are *passing*, not being skipped
        assert fun.count > 0
        if method in MINIMIZE_METHODS:
            assert callback.count > 0
        if method in jac_methods:
            assert grad.count > 0
        if method in hess_methods:
            assert hess.count > 0
        if method in constr_methods:
            assert fun_constraint.count > 0
            assert grad_constraint.count > 0
            np.testing.assert_allclose(res.x[2:], [c0, d0], rtol=rtol)
        else:
>           np.testing.assert_allclose(res.x[2:], 0, atol=atol)
cyipopt/tests/unit/test_scipy_optional.py:308: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
args = (<function assert_allclose.<locals>.compare at 0x7ffcf5691260>, array([-6.84786337e-05, -2.23338511e-05]), array(0))
kwds = {'equal_nan': True, 'err_msg': '', 'header': 'Not equal to tolerance rtol=1e-07, atol=5e-05', 'verbose': True}
    @wraps(func)
    def inner(*args, **kwds):
        with self._recreate_cm():
>           return func(*args, **kwds)
E           AssertionError: 
E           Not equal to tolerance rtol=1e-07, atol=5e-05
E           
E           Mismatched elements: 1 / 2 (50%)
E           Max absolute difference: 6.84786337e-05
E           Max relative difference: inf
E            x: array([-6.847863e-05, -2.233385e-05])
E            y: array(0)
/usr/lib64/python3.12/contextlib.py:81: AssertionError

The “flaky” x86_64 failures are in TestSLSQP::test_minimize_unbounded_approximated; I’ll report more detail on them in a separate issue.

The scipy package is at version 1.11.1. I’m happy to run any tests or add any other details that might be helpful.

musicinmybrain commented 10 months ago

The “flaky” x86_64 failures are in TestSLSQP::test_minimize_unbounded_approximated; I’ll report more detail on them in a separate issue.

https://github.com/mechmotum/cyipopt/issues/238

moorepants commented 9 months ago

A quick note is that we have never officially supported these architectures, but if these failures are only tied to the coblya method then it would more likely be a scipy issue. These new tests are associated with the new support for passing any valid scipy minimize method to our function to make it compatible with the scipy api.

moorepants commented 9 months ago

One more note: we exclude this test from the source release with https://github.com/mechmotum/cyipopt/blob/master/MANIFEST.in#L10. So it isn't clear why you'd be running this test when doing packaging downstream. The PyPi source release should be used for all downstream packaging.

moorepants commented 9 months ago

Sorry, that prior comment doesn't apply to this issue, but would apply to #238

musicinmybrain commented 9 months ago

One more note: we exclude this test from the source release with https://github.com/mechmotum/cyipopt/blob/master/MANIFEST.in#L10. So it isn't clear why you'd be running this test when doing packaging downstream. The PyPi source release should be used for all downstream packaging.

Good point. It looks like the only reason I have been preferring the GitHub archive has been to get the examples/ directory. Currently, I package these along with the documentation, and I run them along with the tests for additional confidence. Neither of these is strictly necessary. On the other hand, would you consider adding the examples/ directory to the PyPI sdist?

moorepants commented 9 months ago

On the other hand, would you consider adding the examples/ directory to the PyPI sdist?

Yes, they are very small so we could do this.