refactor: comment and unify the creation and filling of trackStateProxy (KF)

[AJPfleger]

Makes the generation and filling of the trackStateProxy, look the same at all places, where it occurs. I tried to make it more readable by updating the comments and adding more scopes for temporary variables that are used for the filling.

This should make it more easy to understand what's going on for the next user, because now it is clearer that we are:

1. generating a trackstate
2. fetching values from the propagation
3. filling them in
4. adding flags

[codecov]

Codecov Report

Merging #2499 (f6085fe) into main (b4be75f) will increase coverage by 0.00%.
The diff coverage is 20.63%.

@@           Coverage Diff           @@
##             main    #2499   +/-   ##
=======================================
  Coverage   49.60%   49.60%           
=======================================
  Files         471      471           
  Lines       26687    26695    +8     
  Branches    12280    12284    +4     
=======================================
+ Hits        13238    13242    +4     
  Misses       4747     4747           
- Partials     8702     8706    +4     

Files	Coverage Δ
Core/include/Acts/EventData/TrackStatePropMask.hpp	100.00% <ø> (ø)
Core/include/Acts/TrackFitting/KalmanFitter.hpp	42.47% <25.00%> (+0.08%)	⬆️
...e/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp	20.96% <19.14%> (+2.00%)	⬆️

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

[github-actions]

📊: Physics performance monitoring for f6085fe

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
• ✅ Track Summary CKF ttbar
• ✅ AMVF ttbar
• ✅ AMVF (+grid seeder) ttbar
• ✅ Truth tracking (GSF)
• ✅ Truth tracking
• ✅ Particles inital fatras
• ✅ Particles final fatras
• ✅ Particles inital geant4
• ✅ Particles final geant4

[paulgessinger]

To me this seems like it makes it less readable and harder to follow, also because you lose the comments in between.

But ok if you feel like this is unneeded repetition we can factorize it.

[paulgessinger]

I'm still not convinced this is an improvement. I think we're trading 10 possibly duplicated lines for a function with a very complex signature that does complex logic just to factorize.

[andiwand]

to me this looks a bit like an over generalization. at least with the current code I do not see a big improvement. do you see this making a difference in the GX2F impl or an application on the GSF?

[AJPfleger]

I see that an extra function might be overshooting the purpose.
I still would like to adapt the order line order, since I got always confused when getting to those sections, just to find out, that they are doing the same thing.

Could you have a look again? @paulgessinger @andiwand

[AJPfleger]

We always check for the covariance before setting it. Is there in all 3 cases a chance, that it is missing?

At least from the unitTests I understood, that it is not necessary to check.

if (boundParams.covariance().has_value()) {
  trackStateProxy.predictedCovariance() =
      std::move(*boundParams.covariance());
}

[acts-project-service]

✅ Athena integration test results [e0d26d8]

✅ All tests successful

status	job
🟢	run_workflow_tests_run2_data
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsWorkflow.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateTracks.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsSpacePoints.sh
🟢	run_unit_tests
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsCoreSpacePoints.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateSeeds.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateOrthogonalSeeds.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateClusters.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsPersistifyEDM.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsKfRefitting.sh
🟢	run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsExtrapolationAlgTest.py
🟢	run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsITkTest.py
🟢	run_workflow_tests_run4_mc
🟢	run_workflow_tests_run3_mc
🟢	run_workflow_tests_run3_data