SaneCovariance : rename and allow cases of zero
In the context of ATLASRECTS-7840
- Rename saneCovariance to hasPositiveDiagonal
- Add a method
hasPositiveOrZeroDiagonal
The stepPropagator changes to using hasPositiveOrZeroDiagonal
All else stay the same.
"The issue"
In reality a covariance matrix needs to be Positive SemiDefinite.
Aka a Zero matrix is a valid covariance
This check as I said in the JIRA was introduced by the muon SW. As you can see from the changeLog that is used quite heavily for muons and very little for anything else.
ping @kluit @jojungge @todorova .
For the future
Continuing on my comment on the MR !44413 (merged). Let me ping @tstreble that looks at some of the Eigen facilities for another reason.
One can see https://godbolt.org/z/4ncY3ae3Y .
-
The matrix A in this example (which is the one I mention in the MR by @jojungge Is not positive definite nor positive semi definite . It has an eigenvalue that is <0 .
In theory a matrix that is non positive semi-definite is not valid covariance. -
The zero matrix is positive semi definite. Not positive definite.
-
The matrix B is again neither.
Now
-
The matrix B we can catch as has negative elem in the diagonal
-
The Zero matrix we can handle correcly for the STEP after this change.
There are 2 issues need thinking
-
"we need the matrix to have positive(or 0) diagonal elements" to be positive (semi) definite" but alone this is not enough. That is the issue with A . This will pass as is now . We might want to do something more exact.
-
More practical even for the "weaker" checks we do today. Are we checking covariance for being positive semi definite (it has to be ) or positive definite (it does not have to be) ? This test is all over the Muon software . So something to think about
Let me ping also @jdandoy , @jcatmore , @jchapman , @nstyles