[Math][Minuit2] Add new Strategy to Minuit2 for more precise hesse algorithm and return without forcing positive definite #13109
Conversation
|
Can one of the admins verify this patch? |
|
@phsft-bot build |
|
Starting build on |
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Errors:
|
|
Build failed on ROOT-ubuntu2004/python3. Errors:
|
|
Build failed on mac11/noimt. Errors:
|
|
Build failed on ROOT-debian10-i386/soversion. Errors:
|
|
Build failed on ROOT-performance-centos8-multicore/cxx17. Errors:
|
| @@ -43,7 +43,7 @@ MinimumError MnPosDef::operator()(const MinimumError &e, const MnMachinePrecisio | |||
| } | |||
| // std::cout<<"MnPosDef init matrix= "<<err<<std::endl; | |||
|
|
|||
| double epspdf = std::max(1.e-6, prec.Eps2()); | |||
| double epspdf = std::max(1.e-12, prec.Eps2()); // should this lower bound be configurable? | |||
There was a problem hiding this comment.
I agree this should be a configurable parameter. It is important to decide later in the code (line 85, when looking at the eigenvalues, if the matrix need to be made positive defined by adding some extra values to the diagonal.
It is clear that we higher statistics there will be smaller and smaller eigenvalue of the covariance matrices and the spread of the eigenvalues will increase.
Note that the Fortran implementation of Minuit (and TMInuit) uses 1.E-6 as value for this parameter.
There was a problem hiding this comment.
Thank you Will for this nice improvement.
This PR provides actually two new improvements:
- strategy=3 to have an improved Hessian using a 5 evaluations for the second derivatives of the off-diagonal elements.
- increase tolerance value (
epspdfto decide when the covariance matrix needs to be corrected and made pos defined.
For strategy 3 this PR is not doing two things that are done in strategy 2:
- compute full Hessian in MnSeedGenerator.cxx (see line 214). I think this is good since it is often not very useful
- compute Hessian at the end of the minimization iterations to re-calculate the
elm(line 136 in VariableMetricBuilder.cxx). I think we can probably leave it as it is, the user can always force the Hessian computation.
Another comment: should we maybe also consider extra function evaluations also for computing the diagonal elements of the covariance ? At the moment we use only 2 extra points.
|
Build failed on mac12arm/cxx20. Errors:
|
will-cern
force-pushed
the
minuit2NewStrategy
branch
2 times, most recently
from
54ad7e2
to
d995496
Compare
New Minuit2 strategy for improved Hessian calculation and return without
making positive definite.
This proposed new Strategy in minuit2 is the same migrad behaviour as
Strategy=2 but with the following changes to the Hesse calculation:
* The step and g2 tolerances have been zeroed so that the 7 cycles are
forced in the diagonal-term calculation. This was found to be
necessary in cases where asimov datasets were used for the
minimization and there were very few iterations for the approximate
covariance to be determined from.
* Central finite difference is used for mixed partial derivatives.
This requires 3 extra function evaluations per derivative, but is
necessary in the case of minima where there is high curvature (in
the case of high stats) and the forward finite difference (default)
behaviour leads incorrectly to a non-positive-definite covariance
matrix
* Return the uncorrected covariance matrix, even if it is not positive
definite. This useful for checking just how far from
positive-definiteness the matrix is by being able to examine the
eigenvalues.
Additionally, a lower bound on the precision allowed for the spread of
eigenvalues of the "hessian" correlation matrix (computing a correlation
matrix with the hessian as if it was a covariance matrix) was reduced
from 1e-6 to 1e-12 (see MnHesse.cxx) ... it is not clear why 1e-6 was
the lower bound previously, but current machine precision can beat that
(I get locally 1e-8). I left a comment about whether this lower bound
should be made configurable or not...
This new strategy was tested with a model with high statistics (almost
50 million events) where the migrad minimization was successful but the
hessian was being forced positive definite. With this new Strategy 3 the
hessian is accurate and positive definite in all cases tested.
guitargeek
changed the title
| @@ -1201,7 +1204,43 @@ bool Minuit2Minimizer::Hesse() | |||
| return false; | |||
| } | |||
|
|
|||
| const int strategy = Strategy(); | |||
|
|
|||
| // set strategy and add extra options if needed | |||
There was a problem hiding this comment.
It would be good if the code is not duplicated from Minimize(), but I can also take care of this technicality.
guitargeek
force-pushed
the
minuit2NewStrategy
branch
from
681ae24
to
5971f1a
Compare
|
@phsft-bot build |
|
Starting build on |
|
Build failed on windows10/default. Errors:
And 12 more |
This proposed new Strategy in minuit2 is the same migrad behaviour as Strategy=2 but with the following changes to the Hesse calculation:
Additionally, a lower bound on the precision allowed for the spread of eigenvalues of the "hessian" correlation matrix (computing a correlation matrix with the hessian as if it was a covariance matrix) was reduced from 1e-6 to 1e-12 (see MnHesse.cxx) ... it is not clear why 1e-6 was the lower bound previously, but current machine precision can beat that (I get locally 1e-8). I left a comment about whether this lower bound should be made configurable or not...
This new strategy was tested with a model with high statistics (almost 50 million events) where the migrad minimization was successful but the hessian was being forced positive definite. With this new Strategy 3 the hessian is accurate and positive definite in all cases tested.