From ad24668bc72912919ebc36de4eff26c8dae4ebd0 Mon Sep 17 00:00:00 2001
From: Stephen Nicholas Swatman <stephen.nicholas.swatman@cern.ch>
Date: Fri, 9 Oct 2020 15:36:06 +0200
Subject: [PATCH] Guard against invalid pattern parameter covariance
The `Trk::PatternTrackParameters` class contains, like the other track
parameter classes, an optional covariance matrix. In other track
parameter classes, the nullability of the covariance matrix is modelled
using pointers. The pattern track parameters, on the contrary, always
contain a covariance matrix, and use a boolean flag to indicate whether
the data stored in this covariance matrix data member is valid or not.
The pattern track parameters class provides a `newCovarianceMatrix`
method which updates the covariance matrix of the current instance
according to a Jacobian matrix and the covariance matrix of a second set
of track parameters. The implementation of this method assumes that the
covariance matrix stored in the second set of parameters is valid, and
also marks the covariance of the object on which it is called as valid.
It follows from this that the method should only be called if the second
parameter has a valid and properly initialized covariance matrix.
I was able to find two cases in which the validity of the covariance
matrix is not checked: in the Runge-Kutta propagator and in the SI
trajectory element. This may lead to unsanitary behaviour, and I believe
this to be a bug in terms of physics performance. Investigation reveals
that there are indeed cases where this method is called for parameters
with invalid covariance matrices.
To counteract this effect, I have added additional safeguards to prevent
the execution of the aforementioned method in cases where the second set
of track parameters has an invalid covariance matrix, thereby ensuring
that invalid covariance information is not propagated to other parts of
the code base.
This commit introduces slight changes in physics output, but these
changes are accounted for and I believe them to be improvements over the
old output.
---
.../SiSPSeededTrackFinderData/src/SiTrajectoryElement_xk.cxx | 4 ++++
.../TrkExRungeKuttaPropagator/src/RungeKuttaPropagator.cxx | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/src/SiTrajectoryElement_xk.cxx b/InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/src/SiTrajectoryElement_xk.cxx
index 852e8a227cad..7802977aeb0b 100644
--- a/InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/src/SiTrajectoryElement_xk.cxx
+++ b/InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/src/SiTrajectoryElement_xk.cxx
@@ -1111,6 +1111,10 @@ bool InDet::SiTrajectoryElement_xk::transformGlobalToPlane
Jac[19] =(C*P[40]-s4*C44)*n; // dThe/dCM
Jac[20] = 1.; // dCM /dCM
+ if (!Ta.iscovariance()) {
+ return false;
+ }
+
Tb.newCovarianceMatrix(Ta,Jac);
const double* t = &Tb.cov()[0];
if(t[0]<=0. || t[2]<=0. || t[5]<=0. || t[9]<=0. || t[14]<=0.) return false;
diff --git a/Tracking/TrkExtrapolation/TrkExRungeKuttaPropagator/src/RungeKuttaPropagator.cxx b/Tracking/TrkExtrapolation/TrkExRungeKuttaPropagator/src/RungeKuttaPropagator.cxx
index 30b15b4ff69b..1522401f48c2 100755
--- a/Tracking/TrkExtrapolation/TrkExRungeKuttaPropagator/src/RungeKuttaPropagator.cxx
+++ b/Tracking/TrkExtrapolation/TrkExRungeKuttaPropagator/src/RungeKuttaPropagator.cxx
@@ -1578,6 +1578,10 @@ bool Trk::RungeKuttaPropagator::propagateRungeKutta
//
Tb.setParameters(&Su,p);
if(useJac) {
+ if (!Ta.iscovariance()) {
+ return false;
+ }
+
Tb.newCovarianceMatrix(Ta,Jac);
const double* cv = Tb.cov();
if( cv[0]<=0. || cv[2]<=0. || cv[5]<=0. || cv[9]<=0. || cv[14]<=0.) return false;
--
GitLab