pyhf
pyhf copied to clipboard
"Optimization terminated successfully." when HESSE fails after MIGRAD
Summary
When HESSE fails after MIGRAD with the minuit
backend, the resulting error is accompanied by a misleading "Optimization terminated successfully." message. This happens since the status is queried before HESSE is run (which can turn a valid into an invalid minimum, related: https://github.com/scikit-hep/iminuit/issues/746):
https://github.com/scikit-hep/pyhf/blob/2cd8c0be20b6004a3fdb5305afd13d970589f547/src/pyhf/optimize/opt_minuit.py#L123-L125
OS / Environment
n/a
Steps to Reproduce
This uses a workspace from https://www.hepdata.net/record/resource/2258588?landing_page=True.
curl -OJLH "Accept: application/x-tar" https://doi.org/10.17182/hepdata.100351.v2/r1
tar -xvf likelihood_ttbarZ_13TeV.tar.xz
pyhf fit likelihood_ttbarZ_13TeV/3L_workspace.json --optimizer minuit
File Upload (optional)
No response
Expected Results
The error message should reflect that HESSE failed.
Actual Results
File "[...]/pyhf/src/pyhf/cli/infer.py", line 113, in fit
fit_result = mle.fit(data, model, return_fitted_val=value)
File "[...]/pyhf/src/pyhf/infer/mle.py", line 131, in fit
return opt.minimize(
File "[...]/pyhf/src/pyhf/optimize/mixins.py", line 185, in minimize
result = self._internal_minimize(
File "[...]/pyhf/src/pyhf/optimize/mixins.py", line 65, in _internal_minimize
raise exceptions.FailedMinimization(result)
pyhf.exceptions.FailedMinimization: Optimization terminated successfully.
pyhf Version
pyhf, version 0.7.0rc2.dev3
Code of Conduct
- [X] I agree to follow the Code of Conduct
Hi @alexander-held! Would something like this work here?
try:
minimizer.hesse()
hess_inv = minimizer.covariance
corr = hess_inv.correlation()
unc = minimizer.errors
message = "Hesse converges"
except:
message = "Hesse fails."
I assume that would work, but we might need one of the core developers to comment on how exactly the FailedMinimization
exception works and where it is raised. It might also be possible to check minimizer.valid
again after the minimizer.hesse()
call.
Not sure what you want from the logic. When hesse
fails, that's because the minimizer.valid == True
right before calling it, so we'd probably want to just update minuit to have the message that Hesse failed.
One thing that could be done is to raise separate exceptions for hesse and migrad, eg FailedMigrad
and FailedHesse
to differentiate betwen whatever's happening.
Related to what @kratsg was saying, looking at
https://github.com/scikit-hep/pyhf/blob/1191f6676002b26bbd751c1b8e116400baaed25e/src/pyhf/optimize/opt_minuit.py#L136-L138
this final minimizer.hesse()
call can result in minimizer.valid=False
. This is again why https://github.com/scikit-hep/iminuit/issues/746 is still open. While we could guess at the cause by doing something like
if minimizer.valid:
# Extra call to hesse() after migrad() is always needed for good error estimates.
# If you pass a user-provided gradient to MINUIT, convergence is faster.
minimizer.hesse()
if not minimizer.valid:
message = "Optimization failed."
fmin = minimizer.fmin
if not fmin.has_made_posdef_covar:
message += (
" Failed to make the covariance matrix positive definite."
)
given how HESSE works, it would probably be better to try to go spend time working with Hans to figure out how to deal with https://github.com/scikit-hep/iminuit/issues/746 more appropriately.
Related to what @kratsg was saying, looking at
https://github.com/scikit-hep/pyhf/blob/1191f6676002b26bbd751c1b8e116400baaed25e/src/pyhf/optimize/opt_minuit.py#L136-L138
this final
minimizer.hesse()
call can result inminimizer.valid=False
. This is again why scikit-hep/iminuit#746 is still open. While we could guess at the cause by doing something likeif minimizer.valid: # Extra call to hesse() after migrad() is always needed for good error estimates. # If you pass a user-provided gradient to MINUIT, convergence is faster. minimizer.hesse() if not minimizer.valid: message = "Optimization failed." fmin = minimizer.fmin if not fmin.has_made_posdef_covar: message += ( " Failed to make the covariance matrix positive definite." )
given how HESSE works, it would probably be better to try to go spend time working with Hans to figure out how to deal with scikit-hep/iminuit#746 more appropriately.
In this implementation, wouldn't it make more sens to use has_posdef_covar
instead of has_made_posdef_covar
?
In this implementation, wouldn't it make more sens to use
has_posdef_covar
instead ofhas_made_posdef_covar
?
That's probably fine, but my point is more that this is trying to handle something that is out of scope for pyhf
and should be handeled in iminuit
generally.
I think some of this can be done in iminuit
, the main thing in my view is not having a "optimization successful" message when the minimum is invalid. Maybe the easiest would be moving this whole block
https://github.com/scikit-hep/pyhf/blob/2cd8c0be20b6004a3fdb5305afd13d970589f547/src/pyhf/optimize/opt_minuit.py#L123-L130
below the Hesse section
https://github.com/scikit-hep/pyhf/blob/2cd8c0be20b6004a3fdb5305afd13d970589f547/src/pyhf/optimize/opt_minuit.py#L135-L140
and (possibly) before Hesse only add something about Migrad to the message?
I think some of this can be done in
iminuit
, the main thing in my view is not having a "optimization successful" message when the minimum is invalid. Maybe the easiest would be moving this whole blockhttps://github.com/scikit-hep/pyhf/blob/2cd8c0be20b6004a3fdb5305afd13d970589f547/src/pyhf/optimize/opt_minuit.py#L123-L130
below the Hesse section
https://github.com/scikit-hep/pyhf/blob/2cd8c0be20b6004a3fdb5305afd13d970589f547/src/pyhf/optimize/opt_minuit.py#L135-L140
and (possibly) before Hesse only add something about Migrad to the message?
This is what I was trying to do with my code. However, I am a bit sceptical about the message "Optimisation terminated successfully" in thee first place, as this will probably only appear whenever FailedMinimization
exception has been invoked, in which case it'll look misleading.