-
Notifications
You must be signed in to change notification settings - Fork 173
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
Common shared measurement computation? #3618
Comments
I think one problem is that apparently do not set this flag in the ambi solvers. We should definitely fix this. For the track finding I am not sure how useful it is as there might be duplicated tracks just by seed duplication. Anyways I think this functionality should be moved to Core, most likely into |
As proposed in the dev meeting: Let's gather more information until Friday and take a decision then. |
Discussed briefly with @paulgessinger , we need to do this at the end of the track finding pass otherwise the track collection is immutable and we can't properly tag the state. So, plan of action is to move this function to core, keep the call where it is in the examples CKF pass, & remove it from the ambiguity solvers. PR expected soon(TM) @AJPfleger you can remove the needs decision label? |
Having it in core definitely makes sense. At the end of the track finding I am not sure because I don't know if we really need this flag before the ambi is called anyways. But in ACTS world that could be a flag in the track finding as it is now I think. In case of the ambi solvers I would still think that we could have a shortcut since they will have some similar data lying around. But the user can also just call your helper function at the end of the ambi for now. |
I can benchmark both scenarios -- end of CKF or start of ambi, if that's useful |
This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs. |
in TrackFindingAlgorithm in the examples (here) the
computeSharedHits
method is prefaced with the following comment:I think it can make sense to have this decoupled from the ambiguity solving algorithms -- I'm happy to work on this but I guess we should discuss.
@andiwand any thoughts (git-blame points to you for this comment)?
Another point to discuss -- we might want to add another entry to the
TrackStateFlag
enum, something likeMergedHitFlag
, which is a shared hit but that's identified as a "true" (or allowed) multi-particle state.The text was updated successfully, but these errors were encountered: