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

refactor: Remove duplicatedPredictor from CKFPerformanceWriter #2964

Merged

Conversation

andiwand
Copy link
Contributor

This came to my attention while working on #2904. I do not see this used anywhere and too specific for a general purpose tracking performance writer.

If we want to classify duplicated tracks we should do that before writing the performance in a specific deduplication algorithm (like the ambiguity resolution

).

@andiwand andiwand added this to the next milestone Feb 19, 2024
@github-actions github-actions bot added the Component - Examples Affects the Examples module label Feb 19, 2024
@paulgessinger
Copy link
Member

@Corentin-Allaire did you add this?

@Corentin-Allaire
Copy link
Contributor

@Corentin-Allaire did you add this?

No that is not from me (I don't do duplicate predication anywhere). I do remember someone (a student?) working on that a very long time ago (I want to say with Moritz but I am not 100% sure)

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8f19749) 48.78% compared to head (e136c71) 48.78%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2964   +/-   ##
=======================================
  Coverage   48.78%   48.78%           
=======================================
  Files         493      493           
  Lines       28914    28914           
  Branches    13752    13752           
=======================================
  Hits        14105    14105           
  Misses       4909     4909           
  Partials     9900     9900           

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

Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

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

Let's kill it.

@kodiakhq kodiakhq bot merged commit d5d0890 into acts-project:main Feb 20, 2024
54 checks passed
@andiwand andiwand deleted the remove-ckfperfwriter-duplicatedPredictor branch March 1, 2024 15:33
@paulgessinger paulgessinger modified the milestones: next, v33.0.0 Mar 6, 2024
kodiakhq bot pushed a commit that referenced this pull request Mar 19, 2024
An attempt to move to a central truth matching in the Examples framework. Currently all our writers have a separate truth matching implementation with some amount of common code. I propose to move the truth matching to its own algorithm and then use the output as an input for the writers.

This also allows for simple truth matching in case of truth tracking to be specialized and for a common mechanism for how we infer the truth information. E.g. some algorithms right now expect particles and tracks in a 1:1 correspondence in the input containers which is not achievable with CKF tracking.

blocked by
- #2964
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
…cts-project#2964)

This came to my attention while working on acts-project#2904. I do not see this used anywhere and too specific for a general purpose tracking performance writer.

If we want to classify duplicated tracks we should do that before writing the performance in a specific deduplication algorithm (like the ambiguity resolution https://github.com/acts-project/acts/blob/84c039fd8bd7cf3a7a18d5d66b1e662aa5e2e9f3/Examples/Algorithms/AmbiguityResolution/include/ActsExamples/AmbiguityResolution/GreedyAmbiguityResolutionAlgorithm.hpp#L36).
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
…#2904)

An attempt to move to a central truth matching in the Examples framework. Currently all our writers have a separate truth matching implementation with some amount of common code. I propose to move the truth matching to its own algorithm and then use the output as an input for the writers.

This also allows for simple truth matching in case of truth tracking to be specialized and for a common mechanism for how we infer the truth information. E.g. some algorithms right now expect particles and tracks in a 1:1 correspondence in the input containers which is not achievable with CKF tracking.

blocked by
- acts-project#2964
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
…cts-project#2964)

This came to my attention while working on acts-project#2904. I do not see this used anywhere and too specific for a general purpose tracking performance writer.

If we want to classify duplicated tracks we should do that before writing the performance in a specific deduplication algorithm (like the ambiguity resolution https://github.com/acts-project/acts/blob/84c039fd8bd7cf3a7a18d5d66b1e662aa5e2e9f3/Examples/Algorithms/AmbiguityResolution/include/ActsExamples/AmbiguityResolution/GreedyAmbiguityResolutionAlgorithm.hpp#L36).
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
…#2904)

An attempt to move to a central truth matching in the Examples framework. Currently all our writers have a separate truth matching implementation with some amount of common code. I propose to move the truth matching to its own algorithm and then use the output as an input for the writers.

This also allows for simple truth matching in case of truth tracking to be specialized and for a common mechanism for how we infer the truth information. E.g. some algorithms right now expect particles and tracks in a 1:1 correspondence in the input containers which is not achievable with CKF tracking.

blocked by
- acts-project#2964
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Examples Affects the Examples module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants