Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: computation of shared measurement in CKF #898

Merged
merged 36 commits into from
Sep 1, 2021

Conversation

CarloVarni
Copy link
Collaborator

Computation of shared measurements in CKF. After track reconstruction, the code runs on all reconstructed tracks and takes into account the measurements used in each track by accessing the measurement index contained in the source link. If the same measurement is used by more than one track, it is flagged as a SharedHit.

The measurement will have both the Measurement and the Shared flag, so no change in measurement-related variables is expected.

In attachment the number of Shared Measurement distribution as a function of eta and pt.
shared_vs_eta.pdf
shared_vs_pt.pdf

Here is also the number of shared hits for tracks: nSharedHits_raw.pdf
Concerning the color legend:

  • black=good track
  • red=duplicate track
  • green=fake track

the labelling is taken from CsvMultiTrajectoryWriter.cpp

The high number of shared measurements is a result of the CKF reconstructing several tracks from the same truth particles. These reconstructed tracks shared almost all the measurements.
For instance this particular case in which three different tracks are reconstructed
recoParticlesFor_78019200.pdf:

[Measurement indexes] [track type]
[6851, 10338, 16437, 16430, 25254, 25320, 31900, 32675, 33246, 33979, 40070] [duplicate]
[6851, 10338, 16435, 16430, 17039, 25254, 25320, 31900, 32675, 33246, 33979, 40070] [duplicate]
[6851, 10338, 16435, 16429, 17039, 25254, 25320, 31900, 32675, 33246, 33979, 40070] [good]

Pinging @XiaocongAi @paulgessinger @asalzburger

@gagnonlg gagnonlg added the 🚧 WIP Work-in-progress label Jul 26, 2021
@CarloVarni
Copy link
Collaborator Author

Here is the shared hits distribution if we recompute that quantity taking into account the truth particle information (i.e. measurements used only by reconstructed tracks from the same truth particle are not labelled as shared): nSharedHits_recomputed.pdf

@codecov
Copy link

codecov bot commented Jul 26, 2021

Codecov Report

Merging #898 (fad124c) into main (43c4f42) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #898   +/-   ##
=======================================
  Coverage   48.48%   48.48%           
=======================================
  Files         334      334           
  Lines       17091    17091           
  Branches     8081     8081           
=======================================
  Hits         8287     8287           
  Misses       3099     3099           
  Partials     5705     5705           
Impacted Files Coverage Δ
Core/include/Acts/EventData/MultiTrajectory.hpp 70.53% <ø> (ø)
.../include/Acts/EventData/MultiTrajectoryHelpers.hpp 50.00% <ø> (ø)
...de/Acts/TrackFinding/CombinatorialKalmanFilter.hpp 29.94% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43c4f42...fad124c. Read the comment docs.

@paulgessinger
Copy link
Member

Is this WIP or ready to review?

@CarloVarni
Copy link
Collaborator Author

Is this WIP or ready to review?

Hi @paulgessinger , I think this PR is ready to review.

@paulgessinger paulgessinger removed the 🚧 WIP Work-in-progress label Jul 30, 2021
@paulgessinger paulgessinger added this to the next milestone Aug 10, 2021
Copy link
Contributor

@XiaocongAi XiaocongAi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @CarloVarni , I have some further comments, which should be easy to address. I will approve once they are addressed.

Copy link
Contributor

@XiaocongAi XiaocongAi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @CarloVarni , thank you for the changes. It looks good to me.

@asalzburger
Copy link
Contributor

Updating this ...

@XiaocongAi XiaocongAi self-requested a review August 24, 2021 15:18
@paulgessinger
Copy link
Member

I think we'll have to wait for one more round of CI before merging.

@paulgessinger paulgessinger enabled auto-merge (squash) September 1, 2021 08:27
@paulgessinger paulgessinger merged commit 8f1f47b into acts-project:main Sep 1, 2021
@paulgessinger paulgessinger modified the milestones: next, v13.0.0 Sep 21, 2021
@CarloVarni CarloVarni deleted the ComputeSharedHitsInCKF branch October 23, 2021 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants