[PWGCF] Adding the option to reject the inner or the outer ring of the FT0 detectors. Also adding the option to remap the dead channels of the FT0 detectors and fill them with the amplitudes of the mirrored channels of the detector #14265
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,6 +107,12 @@ | |
| O2_DEFINE_CONFIGURABLE(cfgLocalEfficiency, bool, false, "Use local efficiency object") | ||
| O2_DEFINE_CONFIGURABLE(cfgUseEventWeights, bool, false, "Use event weights for mixed event") | ||
| O2_DEFINE_CONFIGURABLE(cfgDrawEtaPhiDis, bool, false, "draw eta-phi distribution for detectors in used") | ||
| 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(cfgRemapFT0ADeadChannels, bool, false, "Remap FT0A channels 60, 61, 62, 63 to amplitudes from 28,30,29,31 respectively") | ||
| O2_DEFINE_CONFIGURABLE(cfgRemapFT0CDeadChannels, bool, false, "Remap FT0C channels 177->145, 176->144, 178->146, 179->147, 139->115") | ||
| struct : ConfigurableGroup { | ||
| O2_DEFINE_CONFIGURABLE(cfgMultCentHighCutFunction, std::string, "[0] + [1]*x + [2]*x*x + [3]*x*x*x + [4]*x*x*x*x + 10.*([5] + [6]*x + [7]*x*x + [8]*x*x*x + [9]*x*x*x*x)", "Functional for multiplicity correlation cut"); | ||
| O2_DEFINE_CONFIGURABLE(cfgMultCentLowCutFunction, std::string, "[0] + [1]*x + [2]*x*x + [3]*x*x*x + [4]*x*x*x*x - 3.*([5] + [6]*x + [7]*x*x + [8]*x*x*x + [9]*x*x*x*x)", "Functional for multiplicity correlation cut"); | ||
|
|
@@ -236,6 +242,16 @@ | |
| kTOF, | ||
| kITS | ||
| }; | ||
| enum DetectorChannels { | ||
| kFT0AInnerRingMin = 0, | ||
| kFT0AInnerRingMax = 31, | ||
| kFT0AOuterRingMin = 32, | ||
| kFT0AOuterRingMax = 95, | ||
| kFT0CInnerRingMin = 96, | ||
| kFT0CInnerRingMax = 143, | ||
| kFT0COuterRingMin = 144, | ||
| kFT0COuterRingMax = 207 | ||
| }; | ||
| std::array<float, 6> tofNsigmaCut; | ||
| std::array<float, 6> itsNsigmaCut; | ||
| std::array<float, 6> tpcNsigmaCut; | ||
|
|
@@ -639,12 +655,38 @@ | |
| id = ft0.channelC()[iCh]; | ||
| id = id + Ft0IndexA; | ||
| ampl = ft0.amplitudeC()[iCh]; | ||
| if (cfgRemapFT0CDeadChannels) { | ||
| if (id == 177) | ||
| ampl = ft0.amplitudeC()[145]; | ||
| else if (id == 176) | ||
| ampl = ft0.amplitudeC()[144]; | ||
| else if (id == 178) | ||
| ampl = ft0.amplitudeC()[146]; | ||
| else if (id == 179) | ||
| ampl = ft0.amplitudeC()[147]; | ||
| else if (id == 139) | ||
| ampl = ft0.amplitudeC()[115]; | ||
| } | ||
|
Comment on lines
+658
to
+669
There was a problem hiding this comment. The remapping logic for FT0C dead channels appears to have a bug. The variable 'id' is computed as 'ft0.channelC()[iCh] + Ft0IndexA' (line 656), so id values are offset by 96. However, when checking conditions like 'id == 177', the code then accesses 'ft0.amplitudeC()[145]', which uses 145 as a direct array index. This is problematic because:
The same issue affects all the remapping conditions in this block (lines 659-668). Please verify the intended remapping logic and ensure array indices are correctly calculated.
Comment on lines
+658
to
+669
There was a problem hiding this comment. The remapping logic uses hard-coded magic numbers (channel IDs and array indices) without explanation or named constants. This makes the code difficult to understand and maintain. Consider:
This will improve code readability and make it easier to update the channel mapping if needed. |
||
| if ((cfgRejectFT0CInside && (id >= kFT0CInnerRingMin && id <= kFT0CInnerRingMax)) || (cfgRejectFT0COutside && (id >= kFT0COuterRingMin && id <= kFT0COuterRingMax))) | ||
| ampl = 0.; | ||
|
Comment on lines
+670
to
+671
There was a problem hiding this comment. The rejection logic sets amplitude to 0 for channels in the rejected rings, but this happens after the dead channel remapping. This order of operations means that if a dead channel is in a rejected ring, it will first be remapped to a mirror channel's amplitude, and then that amplitude will be set to 0. Consider whether the intended behavior is:
If the current order is intentional, please add a comment explaining this behavior. If not, swap the order of these operations. |
||
| 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 (cfgRemapFT0ADeadChannels) { | ||
| if (id == 60) | ||
| ampl = ft0.amplitudeA()[92]; | ||
| else if (id == 61) | ||
| ampl = ft0.amplitudeA()[93]; | ||
| else if (id == 62) | ||
| ampl = ft0.amplitudeA()[94]; | ||
| else if (id == 63) | ||
| ampl = ft0.amplitudeA()[95]; | ||
| } | ||
|
Comment on lines
+678
to
+687
There was a problem hiding this comment. Similar to the FT0C remapping, this code uses hard-coded magic numbers without explanation. Consider defining named constants for the dead channel IDs (60, 61, 62, 63) and the source channel indices (92, 93, 94, 95), along with comments explaining the rationale for the remapping. This will improve code maintainability and readability.
Comment on lines
+678
to
+687
There was a problem hiding this comment. Similar to the FT0C remapping, the FT0A remapping logic appears to access array indices directly without accounting for the potential mismatch between channel IDs and array indices. The code checks 'id' values (60-63) and then directly accesses 'ft0.amplitudeA()[92]', 'ft0.amplitudeA()[93]', etc. If 'ft0.channelA()[iCh]' returns channel IDs that may not be contiguous or 0-indexed, then accessing 'ft0.amplitudeA()[92]' may not correspond to channel 92. Please verify that the array indices used (92, 93, 94, 95) correctly correspond to the intended source channels for remapping. |
||
| if ((cfgRejectFT0AInside && (id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax)) || (cfgRejectFT0AOutside && (id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax))) | ||
| ampl = 0.; | ||
|
Comment on lines
+688
to
+689
There was a problem hiding this comment. Same as the FT0C case: the rejection logic sets amplitude to 0 after the dead channel remapping. Consider whether this order of operations is intentional and add a clarifying comment, or swap the order if the rejection should happen before remapping. |
||
| registry.fill(HIST("FT0Amp"), id, ampl); | ||
| ampl = ampl / cstFT0RelGain[iCh]; | ||
| registry.fill(HIST("FT0AmpCorrect"), id, ampl); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration description states that FT0A channels 60, 61, 62, 63 should be remapped to amplitudes from channels 28, 30, 29, 31 respectively. However, the implementation remaps them to channels 92, 93, 94, 95. This inconsistency between the documentation and implementation will confuse users. Please verify which channel mapping is correct and update either the code or the description accordingly.