[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

[MartijnLaarhoven]

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

[github-actions]

O2 linter results: ❌ 9 errors, ⚠️ 0 warnings, 🔕 0 disabled

[Copilot]

Pull request overview

This PR adds functionality to handle dead channels in the FT0A and FT0C detectors and to reject specific detector rings in long-range dihadron correlation analysis. The changes introduce configuration options for ring rejection and dead channel remapping, along with an enum defining the channel ranges for inner and outer rings.

Key Changes

• Added 6 new configurable boolean flags to control rejection of inner/outer rings and remapping of dead channels for FT0A and FT0C detectors
• Introduced DetectorChannels enum to define the channel ID ranges for inner and outer rings of both FT0A and FT0C
• Implemented conditional logic in getChannel() method to remap dead channels to mirror channels and to zero out amplitudes for rejected rings

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

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.

[Copilot]

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:

1. Current: Remap dead channels first, then reject rings (dead channels in rejected rings use mirror amplitudes which are then zeroed)
2. Alternative: Reject rings first, then remap only dead channels that are not in rejected rings

If the current order is intentional, please add a comment explaining this behavior. If not, swap the order of these operations.

[Copilot]

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.

[Copilot]

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.

[Copilot]

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:

1. If the intention is to access channel 145, it should be accessed as 'ft0.amplitudeC()[145 - Ft0IndexA]' or by finding the index where 'ft0.channelC()[index] == 145'.
2. The current logic likely accesses incorrect array indices, causing incorrect remapping behavior.

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.

[Copilot]

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.

[Copilot]

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:

1. Defining named constants for the dead channel IDs (e.g., 'kFT0CDeadChannel1 = 177')
2. Defining named constants for the source channel IDs used for remapping
3. Adding comments explaining why these specific channels are dead and why these particular mirror channels were chosen

This will improve code readability and make it easier to update the channel mapping if needed.