pyhf: iminuit v1.5.0 breaks optimization tests

Description

With the release of iminuit v1.5.0 on 2020-09-17 the nightly tests are failing in test_optim.py. Specifically

https://github.com/scikit-hep/pyhf/blob/8a6ee36da4f566d8a37df01e20201098aa1f8a54/tests/test_optim.py#L47

is failing with errors of

        try:
            assert result.success
        except AssertionError:
            log.error(result)
>           raise exceptions.FailedMinimization(result)
E           pyhf.exceptions.FailedMinimization: Optimization failed. Estimated distance to minimum too large.

src/pyhf/optimize/mixins.py:52: FailedMinimization
------------------------------ Captured log call -------------------------------
ERROR    pyhf.optimize.mixins:mixins.py:51       fun: 15.5887451171875
 hess_inv: array([[1., 1.],
       [1., 1.]])
  message: 'Optimization failed. Estimated distance to minimum too large.'
   minuit: <iminuit._libiminuit.Minuit object at 0x5619c82f90a0>
     nfev: 110
     njev: 0
  success: False
      unc: None
        x: array([0.97325551, 0.91712703])

where the pyhf.exceptions.FailedMinimization being raised comes from the raise exceptions.FailedMinimization(result) in

https://github.com/scikit-hep/pyhf/blob/8a6ee36da4f566d8a37df01e20201098aa1f8a54/src/pyhf/optimize/mixins.py#L31-L53

which are of course coming from

https://github.com/scikit-hep/pyhf/blob/8a6ee36da4f566d8a37df01e20201098aa1f8a54/src/pyhf/optimize/opt_minuit.py#L122-L132

https://github.com/scikit-hep/pyhf/blob/8a6ee36da4f566d8a37df01e20201098aa1f8a54/src/pyhf/optimize/opt_minuit.py#L69

Steps to Reproduce

Run the tests using current master.

iminuit_breaks

To show that this is definitley an issue with iminuit v1.5.0+

$ python -m pip install --upgrade "iminuit<1.5.0"
$ pip list | grep iminuit
iminuit                       1.4.9
$ python -m pytest -sx tests/test_optim.py

passes but

$ python -m pip install --upgrade iminuit
$ pip list | grep iminuit
iminuit                       1.5.1
$ python -m pytest -sx tests/test_optim.py

fails.

Checklist

Run git fetch to get the most up to date version of master
Searched through existing Issues to confirm this is not a duplicate issue
Filled out the Description, Expected Behavior, Actual Behavior, and Steps to Reproduce sections above or have edited/removed them in a way that fully describes the issue

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 31 (31 by maintainers)

Commits related to this issue

fix: Restrict iminuit to v1.4.X releases (#1070) * Restrict iminuit to releases in the v1.4.X range for X >= 3 - iminuit v1.5.0 breaks pyhf for 32b TensorFlow and PyTorch - c.f. Issue #1069 — committed to scikit-hep/pyhf by matthewfeickert 4 years ago

Most upvoted comments

No it is fine. Glad to hear that fixed it.

HDembinski on Sep 23, 2020

We likely need to patch pyhf to set the 32b float mode when running over 32b. This may be the last piece needed on our side. I think 3 is enough, but not sure.

kratsg on Sep 23, 2020

Since I referenced your issue in my PR, it was automatically closed. Please confirm that develop works again.

HDembinski on Sep 22, 2020

Ok, I think I get it now. The original code by Piti called Migrad in a loop until either the call limit is reached or a valid minimum is found. Basically it was overriding the stopping condition and went on to “try harder”. I am not super happy with this idea, because it hides problems such as this, but apparently it was working and people rely on this feature.

We would be happy to have this be a configurable option. At least to have a “try_harder=True” or something like this. Can you point to the code changes that were affecting this? I looked through the linked PR but had a hard time figuring out how it might have changed.

kratsg on Sep 21, 2020

here’s a quick script that runs both precision values:

import pyhf
print(f'pyhf.version   = {pyhf.version.__version__}')
import iminuit
print(f'iminuit.version = {iminuit.version.__version__}')


for precision in ['32b', '64b']:
    print(f'precision = {precision}')
    pyhf.set_backend(pyhf.tensor.pytorch_backend(precision=precision), 'minuit')
    m = pyhf.simplemodels.hepdata_like([50.0], [100.0], [10.0])
    data = pyhf.tensorlib.astensor([125.0] + m.config.auxdata)
    try:
      result, obj = pyhf.infer.mle.fit(data, m, do_grad=False, do_stitch=False, return_result_obj=True)
    except pyhf.exceptions.FailedMinimization as e:
      obj = e.result
    print(obj.minuit.fmin)

$ python minuit.py 
pyhf.version   = 0.5.2
iminuit.version = 1.4.9
precision = 32b
------------------------------------------------------------------
| FCN = 14.69                   |     Ncalls=145 (255 total)     |
| EDM = 3.45e-05 (Goal: 0.0002) |            up = 1.0            |
------------------------------------------------------------------
|  Valid Min.   | Valid Param.  | Above EDM | Reached call limit |
------------------------------------------------------------------
|     True      |     True      |   False   |       False        |
------------------------------------------------------------------
| Hesse failed  |   Has cov.    | Accurate  | Pos. def. | Forced |
------------------------------------------------------------------
|     False     |     True      |   True    |   True    | False  |
------------------------------------------------------------------
precision = 64b
------------------------------------------------------------------
| FCN = 13.11                   |      Ncalls=43 (43 total)      |
| EDM = 6.95e-08 (Goal: 0.0002) |            up = 1.0            |
------------------------------------------------------------------
|  Valid Min.   | Valid Param.  | Above EDM | Reached call limit |
------------------------------------------------------------------
|     True      |     True      |   False   |       False        |
------------------------------------------------------------------
| Hesse failed  |   Has cov.    | Accurate  | Pos. def. | Forced |
------------------------------------------------------------------
|     False     |     True      |   True    |   True    | False  |
------------------------------------------------------------------

versus 1.5.0

$ python minuit.py 
pyhf.version   = 0.5.2
iminuit.version = 1.5.0
precision = 32b
┌──────────────────────────────────┬──────────────────────────────────────┐
│ FCN = 15.59                      │       Ncalls = 99 (110 total)        │
│ EDM = 301 (Goal: 0.0002)         │               up = 1.0               │
├───────────────┬──────────────────┼──────────────────────────────────────┤
│INVALID Minimum│ Valid Parameters │        No Parameters at limit        │
├───────────────┴──────────────────┼──────────────────────────────────────┤
│          ABOVE EDM goal          │           Below call limit           │
├───────────────┬──────────────────┼───────────┬─────────────┬────────────┤
│   Hesse ok    │  Has Covariance  │APPROXIMATE│NOT pos. def.│   FORCED   │
└───────────────┴──────────────────┴───────────┴─────────────┴────────────┘
precision = 64b
┌──────────────────────────────────┬──────────────────────────────────────┐
│ FCN = 13.11                      │        Ncalls = 43 (43 total)        │
│ EDM = 6.95e-08 (Goal: 0.0002)    │               up = 1.0               │
├───────────────┬──────────────────┼──────────────────────────────────────┤
│ Valid Minimum │ Valid Parameters │        No Parameters at limit        │
├───────────────┴──────────────────┼──────────────────────────────────────┤
│          Below EDM goal          │           Below call limit           │
├───────────────┬──────────────────┼───────────┬─────────────┬────────────┤
│   Hesse ok    │  Has Covariance  │ Accurate  │  Pos. def.  │ Not forced │
└───────────────┴──────────────────┴───────────┴─────────────┴────────────┘

kratsg on Sep 21, 2020