refactor: fittedParams -> fittedMomentum in TrackAtVertex struct

[felix-russo]

The output of a vertex fitter is

• a vertex position
• the track momenta at this position

Previously, the track momenta from the vertex fit were saved as GenericTrackParameters, where we set d0 = z0 = 0. For the corresponding covariance matrices, we partly used incorrect values (one cannot, in fact, compute entries like Cov(d0, phi) or Cov(z0, z0) as d0 and z0 are not part of the vertex fitting model).

In this PR, fittedParams are replaced by fittedMomenta (i.e., a 3 vector + its covariance matrix). My aim is

• to make the way the vertex fitter works more obvious and
• to remove the incorrect covariances.

[codecov]

Codecov Report

Merging #2359 (5233280) into main (651ef7a) will increase coverage by 0.10%.
The diff coverage is 57.50%.

@@            Coverage Diff             @@
##             main    #2359      +/-   ##
==========================================
+ Coverage   49.60%   49.70%   +0.10%     
==========================================
  Files         452      452              
  Lines       25515    25467      -48     
  Branches    11701    11650      -51     
==========================================
+ Hits        12656    12658       +2     
+ Misses       4579     4578       -1     
+ Partials     8280     8231      -49     

Files Changed	Coverage Δ
...clude/Acts/Vertexing/AdaptiveMultiVertexFinder.hpp	89.47% <ø> (ø)
...nclude/Acts/Vertexing/KalmanVertexTrackUpdater.ipp	15.78% <25.00%> (+0.86%)	⬆️
...include/Acts/Vertexing/FullBilloirVertexFitter.ipp	33.60% <28.57%> (+5.63%)	⬆️
...e/include/Acts/Vertexing/IterativeVertexFinder.ipp	27.20% <33.33%> (ø)
...clude/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp	40.75% <50.00%> (-0.07%)	⬇️
...ore/include/Acts/Vertexing/KalmanVertexUpdater.ipp	28.57% <66.66%> (ø)
...clude/Acts/Vertexing/AdaptiveMultiVertexFitter.ipp	44.73% <100.00%> (ø)
Core/include/Acts/Vertexing/TrackAtVertex.hpp	100.00% <100.00%> (+33.33%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[github-actions]

📊: Physics performance monitoring for 5233280

Full contents

physmon summary

• ✅ CKF truth_smeared
• ✅ IVF truth_smeared
• ✅ AMVF truth_smeared
• ✅ Track Summary CKF truth_smeared
• ✅ Seeding truth_estimated
• ✅ CKF truth_estimated
• ✅ IVF truth_estimated
• ✅ AMVF truth_estimated
• ✅ Track Summary CKF truth_estimated
• ✅ Seeding seeded
• ✅ CKF seeded
• ✅ IVF seeded
• ✅ AMVF seeded
• ✅ AMVF (+grid seeder) seeded
• ✅ Track Summary CKF seeded
• ✅ Seeding orthogonal
• ✅ CKF orthogonal
• ✅ IVF orthogonal
• ✅ AMVF orthogonal
• ✅ Track Summary CKF orthogonal
• ✅ Ambisolver seeded
• ✅ Ambisolver orthogonal
• ✅ Seeding ttbar
• ✅ CKF ttbar
• ✅ Ambisolver
• ✅ AMVF ttbar
• ✅ AMVF (+grid seeder) ttbar
• ✅ Truth tracking (GSF)
• ✅ Truth tracking
• ✅ Particles inital fatras
• ✅ Particles final fatras
• ✅ Particles inital geant4
• ✅ Particles final geant4

[felix-russo]

Explanation of the physmon failure:

• In the old code, fittedParams were set to originalParams upon initialization. For some tracks, the former were never updated during the AMVF vertex fit due to a bug, see Bug: not all tracks have fittedMomentum #2364. Now, fittedMomentum is std::optional, and, instead of not being updated, it remains std::nullopt. This explains why there are less track momenta monitored than in the reference for the AMVF.
• The computation of the covariance of phi was faulty in the old code. This explains why trk_pullPhiFitted now has a wider distribution.

[andiwand]

one conceptional question:

I guess the fitter is only modifying the momentum but not the position of the track right? Do we constrain the position somehow, e.g. the position of the vertex?

Is it still possible to access the full refitted track parameters in some way? I don't have a use case in mind but I guess it could be handy if somebody wants to use them for something.

[paulgessinger]

I guess the fitter is only modifying the momentum but not the position of the track right? Do we constrain the position somehow, e.g. the position of the vertex?

that's the thing (if i understand correctly). previously we would use the vertex position, but that gives an inconsistent parameter set and an inconsistent covariance, which I found confusing / surprising.

[felix-russo]

Thanks a lot for the review!

I guess the fitter is only modifying the momentum but not the position of the track right? Do we constrain the position somehow, e.g. the position of the vertex?

Yes, the model assumes that the position of the track is exactly at the reconstructed vertex.

Is it still possible to access the full refitted track parameters in some way? I don't have a use case in mind but I guess it could be handy if somebody wants to use them for something.

The full refitted track parameters would be (0, 0, phiFitted, thetaFitted, qOverPFitted). I will add a comment to explain this better!

Since the vertex fitting model does not provide covariances for the impact parameters, we previously used the vertex covariance matrix to compute them. For example Cov(d0, d0) was a function of Cov(xVertex, xVertex), Cov(xVertex, yVertex), etc. as Paul mentioned correctly.

One could argue that we could set Cov(d0, d0) = Cov(sqrt(xVertex^2+yVertex^2), sqrt(xVertex^2+yVertex^2)), but we agreed that this would be a bit sketchy. If they want to, users could do this computation themselves since we provide the vertex covariance.

[andiwand]

do we know if this interface is consumed in Athena? if not we can go forward and merge this

[felix-russo]

do we know if this interface is consumed in Athena? if not we can go forward and merge this

Let's get this in for v29!

[acts-project-service]

🔴 Athena integration test results

Build job with this PR failed!

Please investigate the build job for the pipeline!

[paulgessinger]

Hey @felix-russo, @andiwand. I realize now that the output of this is extensively used in Athena.
Maybe we need to discuss this in the ATLAS vertexing meeting before making this change?

In any case, this will need some amount of validation.

[felix-russo]

Hey @felix-russo, @andiwand. I realize now that the output of this is extensively used in Athena.

Oh really? I thought they didn't care about the refitted momenta?

Maybe we need to discuss this in the ATLAS vertexing meeting before making this change?

Egoistically, I would like to get this in as fast as possible to continue working on the negative AMVF variances. However, I realize that it's you who will have to pick up the pieces. So, in the end, it's your call!

In any case, this will need some amount of validation.

I am happy to provide it (please let me know what you would like to see).

[paulgessinger]

In both the IVF and AMVF tool we construct Trk::VxTrackAtVertex from this information. I'm not sure how widely this object is then used afterwards. I don't think we can change the VxTrackAtVertex EDM, so we'll have to recover the information Athena-side.