[PWGCF] Adding an option to reject part of the FT0 detectors#14242
[PWGCF] Adding an option to reject part of the FT0 detectors#14242MartijnLaarhoven wants to merge 9 commits intoAliceO2Group:masterfrom
Conversation
Added option to reject the inside or the outside of the detector ring of FT0
Added option to reject the inside or the outside of the detector ring of FT0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
making the rejection of the rejection of part of the detector more distinctive
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
MartijnLaarhoven
requested review from
alibuild,
ariedel-cern,
glromane,
lauraser,
lgraczykCern,
otonvd,
prchakra,
shouqiye,
victor-gonzalez and
zchochul
as code owners
|
O2 linter results: ❌ 0 errors, |
github-actions
bot
changed the title
There was a problem hiding this comment.
Pull request overview
This pull request adds configuration options to selectively reject channels from specific rings of the FT0A and FT0C detectors in the long-range dihadron correlation analysis. The FT0 (Fast Interaction Trigger) detector has concentric ring structures, and this feature allows researchers to exclude data from either inner or outer rings during analysis.
Key changes:
- Added four new configurable boolean flags to control rejection of inner/outer ring channels for FT0A and FT0C detectors
- Introduced a
DetectorChannelsenum defining the channel ID ranges for inner and outer rings of both detectors - Implemented rejection logic that zeros out amplitudes from selected detector rings during channel processing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AInside, bool, false, "Rejection of inside ring channels of the FT0A detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outside ring channels of the FT0A detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outside ring channels of the FT0C detector") |
There was a problem hiding this comment.
The description says "Rejection of inside ring channels" which could be ambiguous. Consider clarifying whether "inside" refers to the inner radius (closer to the beam pipe) or something else. A more descriptive term like "inner ring" (as used in the enum) would be clearer and more consistent with the code.
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AInside, bool, false, "Rejection of inside ring channels of the FT0A detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outside ring channels of the FT0A detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outside ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AInside, bool, false, "Rejection of inner ring channels of the FT0A detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outer ring channels of the FT0A detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inner ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outer ring channels of the FT0C detector") |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outside ring channels of the FT0A detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outside ring channels of the FT0C detector") |
There was a problem hiding this comment.
The description says "Rejection of outside ring channels" which could be ambiguous. Consider using "outer ring" instead of "outside ring" for clarity and consistency with the enum naming (kFT0AOuterRingMin/Max, kFT0COuterRingMin/Max).
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outside ring channels of the FT0A detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outside ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outer ring channels of the FT0A detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outer ring channels of the FT0C detector") |
| O2_DEFINE_CONFIGURABLE(cfgDrawEtaPhiDis, bool, false, "draw eta-phi distribution for detectors in used") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AInside, bool, false, "Rejection of inside ring channels of the FT0A detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outside ring channels of the FT0A detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") |
There was a problem hiding this comment.
The description says "Rejection of inside ring channels" which could be ambiguous. Consider using "inner ring" instead of "inside ring" for clarity and consistency with the enum naming (kFT0CInnerRingMin/Max).
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inner ring channels of the FT0C detector") |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AInside, bool, false, "Rejection of inside ring channels of the FT0A detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0AOutside, bool, false, "Rejection of outside ring channels of the FT0A detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0CInside, bool, false, "Rejection of inside ring channels of the FT0C detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outside ring channels of the FT0C detector") |
There was a problem hiding this comment.
The description says "Rejection of outside ring channels" which could be ambiguous. Consider using "outer ring" instead of "outside ring" for clarity and consistency with the enum naming (kFT0COuterRingMin/Max).
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outside ring channels of the FT0C detector") | |
| O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outer ring channels of the FT0C detector") |
| if ((cfgRejectFT0CInside && (id >= kFT0CInnerRingMin && id <= kFT0CInnerRingMax)) || (cfgRejectFT0COutside && (id >= kFT0COuterRingMin && id <= kFT0COuterRingMax))) | ||
| ampl = 0.; | ||
| registry.fill(HIST("FT0Amp"), id, ampl); | ||
| ampl = ampl / cstFT0RelGain[iCh]; | ||
| registry.fill(HIST("FT0AmpCorrect"), id, ampl); |
There was a problem hiding this comment.
The rejection logic is applied after the ID offset for FT0C but before the histogram filling. This means rejected channels still have their amplitude zeroed and will be recorded in the histogram with amplitude 0. Consider whether it would be more appropriate to skip these channels entirely (using 'continue' or early return) rather than setting amplitude to 0, especially since the gain correction is still applied to zero amplitude on line 659. This could lead to unnecessary computations and potentially confusing histogram entries.
| if ((cfgRejectFT0AInside && (id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax)) || (cfgRejectFT0AOutside && (id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax))) | ||
| ampl = 0.; | ||
| registry.fill(HIST("FT0Amp"), id, ampl); | ||
| ampl = ampl / cstFT0RelGain[iCh]; | ||
| registry.fill(HIST("FT0AmpCorrect"), id, ampl); |
There was a problem hiding this comment.
The gain correction on line 667 divides amplitude by cstFT0RelGain[iCh]. When amplitude is set to 0 by the rejection logic (line 665), this results in ampl = 0. / cstFT0RelGain[iCh], which is 0 but still performs an unnecessary division operation. Consider checking if amplitude is 0 before applying the gain correction, or better yet, skip rejected channels entirely earlier in the logic.
1 participant
Adding an option to reject part of the FT0 detectors