Skip to content

Conversation

@davidrohr
Copy link
Collaborator

Supersedes #14020

ChSonnabend and others added 2 commits March 12, 2025 19:27
Adapting edge correction

Fixing edge handling

Please consider the following formatting changes

Fix for right edge check
@github-actions
Copy link
Contributor

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass4
async-2023-pp-apass4
async-2024-pp-apass1
async-2022-pp-apass7
async-2024-pp-cpass0
async-2024-PbPb-apass1
async-2024-ppRef-apass1
async-2024-PbPb-apass2
async-2023-PbPb-apass5

@davidrohr davidrohr changed the title Fixing handling of edge clusters Fixing handling of edge clusters - Don't merge yet Mar 12, 2025
wiechula
wiechula previously approved these changes Mar 13, 2025
@wiechula
Copy link
Collaborator

+async-label async-2024-PbPb-apass2,async-2023-PbPb-apass5

@github-actions github-actions bot dismissed wiechula’s stale review March 13, 2025 08:40

Labels updated; please review again.

@davidrohr davidrohr merged commit 46445fa into AliceO2Group:dev Mar 13, 2025
14 of 15 checks passed
@miranov25
Copy link
Contributor

miranov25 commented Mar 13, 2025

Discussion on Cluster Edge Tagging and Bug Fix - for a reference (highlights and re-formatting from mattermost provided by GPT)

David Rohr

IMHO 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 Ivanov

Can we revisit this statement?

@jens Wiechula @alexander Schmah @marian I Ivanov: With @christian Sonnabend, we found out that the clusterizer flags clusters as edge clusters if the peak is within 2 pads of the edge, i.e., also if the peak is the second or second-to-last pad.

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?
Since we assign larger errors to such clusters, it could significantly impact performance.
When and why was this change introduced?
I must say I did not notice it originally.

David Rohr

It 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.
In principle, we use a 5x5 window, so if we are next to the edge pad, we lose part of the window on one side.
However, for many clusters, we don’t lose much charge, so we unnecessarily increase the error.

That said, I think this is a separate discussion from the fix in my PR.
Actually, my PR fixes some of the cases where the current method is clearly wrong, which aligns with what you are saying.

I’d suggest merging this fix, and we can discuss whether we restrict edge flags to only the edge pad separately.

Marian I Ivanov

The cluster shape for normal clusters is usually restricted to 3×3, so the second pad should already be fine.
I remember showing the bias as a function of distance before.

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.
For highly inclined tracks (which are wider), part of the cluster may be removed, but in that case, it does not matter.

  • The error for inclined tracks is already large.
  • It only affects one pad row, and the track is gone afterward.

I assume this should improve acceptance and dE/dx.

David Rohr

How 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 Ivanov

OK. 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.
The difference is also visible in the debug streamers.

David Rohr

Fair enough, but I prefer to make such decisions based on actual data.
I will merge the current fix and prepare something to compare with flagging only the edge pad.

Marian I Ivanov

For now, I consider this modification a bug fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants