fix(gx2f): propagate final covariance for trackstates #2949

AJPfleger · 2024-02-14T16:12:41Z

Issue

I figured out, that the trackstates in the final track contain the wrong covariance. This occurred, while investigating the unbiased pulls. There, some dimensions had a negative covariance.

acts/Examples/Io/Root/src/RootTrackStatesWriter.cpp

Line 456 in 19464fa

if (ipar == eUnbiased && state.hasSmoothed() && state.hasProjector()) {

Reason

After the last update we calculate the covariance of the initial parameters and add this covariance to the final track. However, the last propagation was done with the initial guess for the covariance. Therefore, the wrong covariance got propagated and assigned to each trackstate.

Solution

Propagate an additional time, with the final parameters+covariance.

Future Plans

Since an additional propagation is quite expensive, we should look into the following two ideas:

calculate covariance after each update (matrix inverse)
toggle the re-propagation if we are not interested in the propagated covariance

Notes

This also removes nUpdate from the Actor since it wasn't needed anymore.

Blocked by

Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp

andiwand · 2024-02-14T17:22:22Z

I think it would be good to have some physmon for gx2f like we have it for KF and GSF. otherwise it will be very hard to judge for someone how these changes behave

AJPfleger · 2024-02-14T17:27:26Z

Good point, on the other hand, the gx2f is still in namespace Experimental, so might be confusing if something gives weird physmon outputs?

codecov · 2024-02-14T17:54:42Z

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (6fb49f1) 48.74% compared to head (ef55e41) 48.73%.

Files	Patch %	Lines
...nclude/Acts/TrackFitting/GlobalChiSquareFitter.hpp	44.44%	0 Missing and 10 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2949      +/-   ##
==========================================
- Coverage   48.74%   48.73%   -0.01%     
==========================================
  Files         493      493              
  Lines       28926    28943      +17     
  Branches    13765    13776      +11     
==========================================
+ Hits        14099    14106       +7     
  Misses       4924     4924              
- Partials     9903     9913      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-02-14T18:11:15Z

📊: Physics performance monitoring for `ef55e41`

Full contents

physmon summary

AJPfleger · 2024-02-19T09:19:18Z

Discussion in meeting 19.02.2024

Pros:

Changes are more comprehensible
Nice plots
Easy

Cons:

Almost all gx2f-updates change physmon
Plots need to be updated in each PR

Outcome:

We want to implement it soon, but no preference before or after this PR. Implement in the next 2 weeks.
The corresponding PR is here:

ci(gx2f): add physmon truth tracking for the GX2F #2966

acts-project-service · 2024-02-22T17:10:54Z

✅ Athena integration test results [`ae56406`]

✅ All tests successful

status	job	report
🟢	run_unit_tests
🟢	test_ActsEFTrackFit
🟢	test_ActsBenchmarkWithSpot
🟢	test_ActsConversionWorkflow
🟢	test_ActsWorkflow
🟢	test_ActsValidateAmbiguityResolution
🟢	test_ActsValidateResolvedTracks
🟢	test_ActsValidateTracks
🟢	test_ActsValidateActsCoreSpacePoints
🟢	test_ActsValidateActsSpacePoints
🟢	test_ActsValidateSeeds
🟢	test_ActsValidateOrthogonalSeeds
🟢	test_ActsValidateClusters
🟢	test_ActsPersistifyEDM
🟢	test_ActsGSFRefitting
🟢	test_ActsKfRefitting
🟢	test_ActsExtrapolationAlgTest
🟢	test_ActsITkTest
🟢	run_workflow_tests_run4_mc
🟢	run_workflow_tests_run2_mc
🟢	run_workflow_tests_run2_data
🟢	run_workflow_tests_run3_mc
🟢	run_workflow_tests_run3_data
🟢	run_art_test: test_data18_13TeV_1000evt
🟢	run_art_test: test_ttbarPU40_reco

## What? Lets the `RootTrackStatesWriter.cpp` write unbiased states, even if there are no `smoothed` states. ## Why? The states we are writing are based upon the Kalman Filter formalism. There, we would expect `predicted`, `filtered`, and `smoothed` states. The `smoothed` states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to `predicted`. Until now, the `unbiased` states could only be computed from `smoothed` states. However, it is also for interest for other fitters to investigate the `unbiased` pulls and residuals. The idea for this PR evolved while investigating the track states for the GX2F (PR #2949). Seems to be useful to have this in. ## Notes We need to check for `state.hasCalibrated()`, since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with ``` state.calibratedSize(): 4294967295 template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX) ```

…ct#2999) ## What? Lets the `RootTrackStatesWriter.cpp` write unbiased states, even if there are no `smoothed` states. ## Why? The states we are writing are based upon the Kalman Filter formalism. There, we would expect `predicted`, `filtered`, and `smoothed` states. The `smoothed` states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to `predicted`. Until now, the `unbiased` states could only be computed from `smoothed` states. However, it is also for interest for other fitters to investigate the `unbiased` pulls and residuals. The idea for this PR evolved while investigating the track states for the GX2F (PR acts-project#2949). Seems to be useful to have this in. ## Notes We need to check for `state.hasCalibrated()`, since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with ``` state.calibratedSize(): 4294967295 template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX) ```

) ## Issue I figured out, that the trackstates in the final track contain the wrong covariance. This occurred, while investigating the unbiased pulls. There, some dimensions had a negative covariance. https://github.com/acts-project/acts/blob/19464fa0d24f031083ef3ad3108b74f2b5fa5454/Examples/Io/Root/src/RootTrackStatesWriter.cpp#L456 ## Reason After the last update we calculate the covariance of the initial parameters and add this covariance to the final track. However, the last propagation was done with the initial guess for the covariance. Therefore, the wrong covariance got propagated and assigned to each trackstate. ## Solution Propagate an additional time, with the final parameters+covariance. ## Future Plans Since an additional propagation is quite expensive, we should look into the following two ideas: - calculate covariance after each update (matrix inverse) - toggle the re-propagation if we are not interested in the propagated covariance ## Notes This also removes `nUpdate` from the Actor since it wasn't needed anymore. ## Blocked by - acts-project#2972 - acts-project#2966

…ct#2999) ## What? Lets the `RootTrackStatesWriter.cpp` write unbiased states, even if there are no `smoothed` states. ## Why? The states we are writing are based upon the Kalman Filter formalism. There, we would expect `predicted`, `filtered`, and `smoothed` states. The `smoothed` states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to `predicted`. Until now, the `unbiased` states could only be computed from `smoothed` states. However, it is also for interest for other fitters to investigate the `unbiased` pulls and residuals. The idea for this PR evolved while investigating the track states for the GX2F (PR acts-project#2949). Seems to be useful to have this in. ## Notes We need to check for `state.hasCalibrated()`, since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with ``` state.calibratedSize(): 4294967295 template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX) ```

) ## Issue I figured out, that the trackstates in the final track contain the wrong covariance. This occurred, while investigating the unbiased pulls. There, some dimensions had a negative covariance. https://github.com/acts-project/acts/blob/19464fa0d24f031083ef3ad3108b74f2b5fa5454/Examples/Io/Root/src/RootTrackStatesWriter.cpp#L456 ## Reason After the last update we calculate the covariance of the initial parameters and add this covariance to the final track. However, the last propagation was done with the initial guess for the covariance. Therefore, the wrong covariance got propagated and assigned to each trackstate. ## Solution Propagate an additional time, with the final parameters+covariance. ## Future Plans Since an additional propagation is quite expensive, we should look into the following two ideas: - calculate covariance after each update (matrix inverse) - toggle the re-propagation if we are not interested in the propagated covariance ## Notes This also removes `nUpdate` from the Actor since it wasn't needed anymore. ## Blocked by - acts-project#2972 - acts-project#2966

…ct#2999) ## What? Lets the `RootTrackStatesWriter.cpp` write unbiased states, even if there are no `smoothed` states. ## Why? The states we are writing are based upon the Kalman Filter formalism. There, we would expect `predicted`, `filtered`, and `smoothed` states. The `smoothed` states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to `predicted`. Until now, the `unbiased` states could only be computed from `smoothed` states. However, it is also for interest for other fitters to investigate the `unbiased` pulls and residuals. The idea for this PR evolved while investigating the track states for the GX2F (PR acts-project#2949). Seems to be useful to have this in. ## Notes We need to check for `state.hasCalibrated()`, since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with ``` state.calibratedSize(): 4294967295 template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX) ```

re-propagate

2924a26

AJPfleger added this to the next milestone Feb 14, 2024

remove logging

16fe0fb

github-actions bot added Component - Core Affects the Core module Track Fitting labels Feb 14, 2024

andiwand reviewed Feb 14, 2024

View reviewed changes

Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp Outdated Show resolved Hide resolved

AJPfleger added 2 commits February 14, 2024 17:37

review + fix

ae2cbc2

update parameters

8d8ccb9

AJPfleger mentioned this pull request Feb 21, 2024

ci(gx2f): add physmon truth tracking for the GX2F #2966

Merged

AJPfleger added the 🛑 blocked This item is blocked by another item label Feb 21, 2024

Merge branch 'main' into covtransport

ef55e41

AJPfleger removed the 🛑 blocked This item is blocked by another item label Feb 22, 2024

AJPfleger requested a review from andiwand February 22, 2024 15:35

andiwand approved these changes Feb 22, 2024

View reviewed changes

AJPfleger added the automerge label Feb 22, 2024

kodiakhq bot merged commit ae56406 into acts-project:main Feb 22, 2024
57 checks passed

github-actions bot removed the automerge label Feb 22, 2024

AJPfleger deleted the covtransport branch February 22, 2024 15:52

AJPfleger mentioned this pull request Feb 29, 2024

feat: unbiased trackstates for fitters that do not smooth #2999

Merged

paulgessinger modified the milestones: next, v33.0.0 Mar 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(gx2f): propagate final covariance for trackstates #2949

fix(gx2f): propagate final covariance for trackstates #2949

AJPfleger commented Feb 14, 2024 •

edited

Loading

andiwand commented Feb 14, 2024

AJPfleger commented Feb 14, 2024

codecov bot commented Feb 14, 2024 •

edited

Loading

github-actions bot commented Feb 14, 2024 •

edited

Loading

AJPfleger commented Feb 19, 2024 •

edited

Loading

acts-project-service commented Feb 22, 2024

fix(gx2f): propagate final covariance for trackstates #2949

fix(gx2f): propagate final covariance for trackstates #2949

Conversation

AJPfleger commented Feb 14, 2024 • edited Loading

Issue

Reason

Solution

Future Plans

Notes

Blocked by

andiwand commented Feb 14, 2024

AJPfleger commented Feb 14, 2024

codecov bot commented Feb 14, 2024 • edited Loading

Codecov Report

github-actions bot commented Feb 14, 2024 • edited Loading

📊: Physics performance monitoring for ef55e41

physmon summary

AJPfleger commented Feb 19, 2024 • edited Loading

Discussion in meeting 19.02.2024

Pros:

Cons:

Outcome:

acts-project-service commented Feb 22, 2024

✅ Athena integration test results [ae56406]

✅ All tests successful

AJPfleger commented Feb 14, 2024 •

edited

Loading

codecov bot commented Feb 14, 2024 •

edited

Loading

github-actions bot commented Feb 14, 2024 •

edited

Loading

📊: Physics performance monitoring for `ef55e41`

AJPfleger commented Feb 19, 2024 •

edited

Loading

✅ Athena integration test results [`ae56406`]