-
Notifications
You must be signed in to change notification settings - Fork 483
Fixing handling of edge clusters - Don't merge yet #14051
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
Conversation
Adapting edge correction Fixing edge handling Please consider the following formatting changes Fix for right edge check
|
REQUEST FOR PRODUCTION RELEASES: This will add The following labels are available |
|
+async-label async-2024-PbPb-apass2,async-2023-PbPb-apass5 |
Labels updated; please review again.
Discussion on Cluster Edge Tagging and Bug Fix - for a reference (highlights and re-formatting from mattermost provided by GPT)David RohrIMHO this should be totally safe, except for the case where the edge pad is dead, but the pad next to the edge pad is not dead. Marian I IvanovCan we revisit this statement?
In my Run 1, Run 2 tracking, I tagged clusters only if the maximum was at the edge. Expanding it to the second pad from the edge could indeed be problematic for tracking. Was there any research done on this particular aspect? David RohrIt has always been like this in the Run 3 Clusterizer. I don't know if any research was conducted. I don't remember if the HLT cluster finder in Run 2 did it the same way—we had cluster flags in Run 2 as well. I agree that this is debatable. That said, I think this is a separate discussion from the fix in my PR. I’d suggest merging this fix, and we can discuss whether we restrict edge flags to only the edge pad separately. Marian I IvanovThe cluster shape for normal clusters is usually restricted to 3×3, so the second pad should already be fine. I can check the debug streamers from past reconstructions again. I prefer to fix both problems in the pull request. For small-angle tracks, my Run 1, Run 2 definition was that only tagging the edge pad is the correct approach.
I assume this should improve acceptance and dE/dx. David RohrHow about we merge the fix, as it is already correct, and then compare the performance of tagging only the edge pad by running both codes on the same data and comparing the tracks and dE/dx? Marian I IvanovOK. Can you create two pull requests and add this as an option? I am certain we should use what I just described. I can show the difference at the edges—I have it in my cluster error presentation. David RohrFair enough, but I prefer to make such decisions based on actual data. Marian I IvanovFor now, I consider this modification a bug fix. |
Supersedes #14020