Skip to content

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

  1. "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.

  2. 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

Edited by Christos Anastopoulos

Merge request reports