[PWGCF] Added option to reject the inside or the outside of the detector ring of FT0 #14191
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
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 PR introduces configurable options to reject inner or outer detector rings of the FT0 detector without using magic numbers. It adds two boolean configurables (cfgRejectInsideDetectors and cfgRejectOutsideDetectors) and defines an enum (DetectorChannels) to replace hardcoded channel range values.
Key changes:
- Added enum constants for FT0A and FT0C inner/outer ring channel ranges
- Introduced two configurables to selectively reject detector ring events
- Implemented rejection logic by setting amplitude to 0 for matching channels
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| O2_DEFINE_CONFIGURABLE(cfgRejectOutsideDetectors, bool, false, "Rejection of outside ring events of the FT0 detector") | ||
| O2_DEFINE_CONFIGURABLE(cfgRejectInsideDetectors, bool, false, "Rejection of inside ring events of the FT0 detector") |
There was a problem hiding this comment.
Both cfgRejectInsideDetectors and cfgRejectOutsideDetectors can be enabled simultaneously, which would reject all FT0 channels (both inner and outer rings). The code should validate that these options are mutually exclusive, or document that enabling both is intentional and will reject all channels.
| if ((cfgRejectInsideDetectors && ((id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax) || (id >= kFT0CInnerRingMin && id <= kFT0CInnerRingMax))) || (cfgRejectOutsideDetectors && ((id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax) || (id >= kFT0COuterRingMin && id <= kFT0COuterRingMax)))) | ||
| ampl = 0.; |
There was a problem hiding this comment.
The rejection logic checks both FT0A and FT0C ranges when processing FT0C channels. However, since FT0C channel IDs are offset by 96 (line 652), they will never match the FT0A inner/outer ring ranges (0-31 and 32-95). The condition should only check FT0C ranges (kFT0CInnerRingMin/Max and kFT0COuterRingMin/Max) when fitType is kFT0C.
| if ((cfgRejectInsideDetectors && ((id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax) || (id >= kFT0CInnerRingMin && id <= kFT0CInnerRingMax))) || (cfgRejectOutsideDetectors && ((id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax) || (id >= kFT0COuterRingMin && id <= kFT0COuterRingMax)))) | ||
| ampl = 0.; | ||
| registry.fill(HIST("FT0Amp"), id, ampl); | ||
| ampl = ampl / cstFT0RelGain[iCh]; | ||
| registry.fill(HIST("FT0AmpCorrect"), id, ampl); | ||
| } else if (fitType == kFT0A) { | ||
| id = ft0.channelA()[iCh]; | ||
| ampl = ft0.amplitudeA()[iCh]; | ||
| if ((cfgRejectInsideDetectors && ((id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax) || (id >= kFT0CInnerRingMin && id <= kFT0CInnerRingMax))) || (cfgRejectOutsideDetectors && ((id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax) || (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.
This logic is duplicated between the FT0C and FT0A branches (lines 654-655 and 662-663). Consider extracting the rejection logic into a helper function that takes the detector type as a parameter to avoid duplication and make the code more maintainable.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
Added option to reject the inside or the outside of the detector ring of FT0 without magic numbers