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

Fix EcalVeto MIP Tracking Implementation Issues #1355

Closed
danyi211 opened this issue May 28, 2024 · 4 comments · Fixed by #1356
Closed

Fix EcalVeto MIP Tracking Implementation Issues #1355

danyi211 opened this issue May 28, 2024 · 4 comments · Fixed by #1356
Assignees
Labels

Comments

@danyi211
Copy link
Contributor

danyi211 commented May 28, 2024

Is your feature request related to a problem? Please describe.

Revisiting MIP tracking algorithm in EcalVetoProcessor, some issues are found:

  1. All hits in layer in the front and behind two layers of current hit layer are added to the track, which could potentially lead to large layer gaps in the track itself, see the code here;
  2. When removing hits in a valid track in a loop, after removing one hit in the list, the index of the next hit to remove will decrease by one, no longer the old index, see code here;
  3. In the end of the for loop, index iHit is decremented "so no hits will get skipped by iHit++", but in addition to iHit (current hit), it is possible to remove more hits behind itself according to 1, see code here.

Describe the solution you'd like

  1. Repeatedly find hits in the front two layers with same x & y positions to avoid large gaps in track;
  2. Use another counting variable n_remove to count the number of hits removed, then the new index of the hit to remove will be the old index minus n_remove;
  3. Decrement index by the number of hits removed in any layer behind (i.e. hits with a smaller index than the current hit in the hit list sorted by decreasing layer number) + 1 (current hit itself is removed)

Describe alternatives you've considered
The straight track contains less than 4 hits and further from electron than photon will be rejected (code), not consistent with the MIP tracking note. It could be due to some further development.

Additional context

https://indico.fnal.gov/event/64896/#8-mip-tracking-update-tbc

@danyi211 danyi211 changed the title Fix EcalVeto MIP Tracking Fix EcalVeto MIP Tracking Implementation Issues May 29, 2024
@tomeichlersmith
Copy link
Member

Excellent work! If you are going to be modifying the EcalVetoProcessor, I would request looking into also breaking it up into several processors. The EcalVetoProcessor is currently very bloated and folks often just want some of its parts but not the whole thing. If that is too much, no worries, having a correct MIP tracking implementation would still be helpful.

#1306

@tvami
Copy link
Member

tvami commented May 29, 2024

I think we'll do that next, the idea is to have the physics correct first and then we can make the performance better. Let's do it incrementally so we can identify what's coming from what sources

@EinarElen
Copy link
Contributor

#1166

If you have time to look at the ecal veto components here after all the incremental steps, I think that would be quite useful. If I don't misremember, patching those two ecal-bugs would allow us to start using sanitizers in CI and catching the worst kind of bugs early rather than late.

@tvami
Copy link
Member

tvami commented May 29, 2024

for (int k = 0; k < 6; k++) {

this seems obv to fix

This one
#1166 (comment)
I'll need to read more the code on what's happening here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants