Skip to content

[PWGCF] Adding the option to remap dead channels #14417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MartijnLaarhoven wants to merge 9 commits into from
Closed

[PWGCF] Adding the option to remap dead channels #14417

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
58b1428
Adding option to reject part of the FT0 detectors
MartijnLaarhoven Dec 19, 2025
35066df
Merge pull request #2 from MartijnLaarhoven/MartijnLaarhoven-patch-1
MartijnLaarhoven Dec 19, 2025
3a07128
Merge branch 'AliceO2Group:master' into master
MartijnLaarhoven Jan 7, 2026
95170ad
Adding the option to remap dead channels
MartijnLaarhoven Jan 7, 2026
30dd963
Merge pull request #3 from MartijnLaarhoven/MartijnLaarhoven-patch-2
MartijnLaarhoven Jan 7, 2026
0e130d2
Add files via upload
MartijnLaarhoven Jan 7, 2026
968a017
Merge pull request #4 from MartijnLaarhoven/MartijnLaarhoven-patch-3
MartijnLaarhoven Jan 7, 2026
8d32d5c
Add files via upload
MartijnLaarhoven Jan 7, 2026
8de6992
Merge branch 'AliceO2Group:master' into master
MartijnLaarhoven Jan 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions PWGCF/TwoParticleCorrelations/Tasks/longRangeDihadronCor.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ struct LongRangeDihadronCor {
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, "If true, remap FT0A channels 60-63 to amplitudes from 92-95 respectively")
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The configuration description states "remap FT0A channels 60-63 to amplitudes from 92-95", but the implementation does the opposite: it remaps channels 92-95 to 60-63 (using dead_id = id - 32). Please verify that the implementation matches the intended behavior and update either the code or the description accordingly.

Suggested change
O2_DEFINE_CONFIGURABLE(cfgRemapFT0ADeadChannels, bool, false, "If true, remap FT0A channels 60-63 to amplitudes from 92-95 respectively")
O2_DEFINE_CONFIGURABLE(cfgRemapFT0ADeadChannels, bool, false, "If true, remap FT0A channels 92-95 to amplitudes from 60-63 respectively")

Copilot uses AI. Check for mistakes.
O2_DEFINE_CONFIGURABLE(cfgRemapFT0CDeadChannels, bool, false, "If true, remap FT0C channels 177->145, 176->144, 178->146, 179->147, 139->115")
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The configuration description should document all remapped channels, not just the outer ring. Currently it only mentions "177->145, 176->144, 178->146, 179->147, 139->115", but it should clarify that 139->115 is the inner ring channel mapping (using kFT0CInnerMirror = 24).

Suggested change
O2_DEFINE_CONFIGURABLE(cfgRemapFT0CDeadChannels, bool, false, "If true, remap FT0C channels 177->145, 176->144, 178->146, 179->147, 139->115")
O2_DEFINE_CONFIGURABLE(cfgRemapFT0CDeadChannels, bool, false, "If true, remap FT0C outer ring channels 177->145, 176->144, 178->146, 179->147, and inner ring channel 139->115 (using kFT0CInnerMirror = 24)")

Copilot uses AI. Check for mistakes.
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");
Expand Down Expand Up @@ -250,6 +252,19 @@ struct LongRangeDihadronCor {
kFT0COuterRingMin = 144,
kFT0COuterRingMax = 207
};
enum MirroringConstants {
kFT0AOuterMirror = 32,
kFT0AInnerMirror = 16,
kFT0COuterMirror = 32,
kFT0CInnerMirror = 24
};
enum DeadChannels {
kFT0ARemapChannelStart = 92,
kFT0ARemapChannelEnd = 95,
kFT0CRemapChannelStart = 144,
kFT0CRemapChannelEnd = 147,
kFT0CRemapChannelInnerRing = 115
};
std::array<float, 6> tofNsigmaCut;
std::array<float, 6> itsNsigmaCut;
std::array<float, 6> tpcNsigmaCut;
Expand Down Expand Up @@ -653,6 +668,19 @@ struct LongRangeDihadronCor {
id = ft0.channelC()[iCh];
id = id + Ft0IndexA;
ampl = ft0.amplitudeC()[iCh];
if (cfgRemapFT0CDeadChannels) {
if (id == kFT0CRemapChannelInnerRing) {
int dead_id = id + kFT0CInnerMirror;
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name dead_id suggests this represents a dead channel ID that should receive data, but the implementation fills histogram data for dead_id using amplitude from the live channel id. This naming is confusing - consider renaming to mirror_id or remap_target_id to clarify that this is the target channel receiving the remapped amplitude.

Copilot uses AI. Check for mistakes.
registry.fill(HIST("FT0Amp"), dead_id, ampl);
ampl = ampl / cstFT0RelGain[iCh];
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect array indexing: cstFT0RelGain[iCh] uses the loop index iCh, but cstFT0RelGain is indexed by channel ID (0-207), not by the position in the sparse channelC() array. This should be cstFT0RelGain[dead_id] since dead_id is the actual channel ID. The same issue affects lines 680 and 696.

Copilot uses AI. Check for mistakes.
registry.fill(HIST("FT0AmpCorrect"), dead_id, ampl);
} else if (id >= kFT0CRemapChannelStart && id <= kFT0CRemapChannelEnd) {
int dead_id = id + kFT0COuterMirror;
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name dead_id suggests this represents a dead channel ID that should receive data, but the implementation fills histogram data for dead_id using amplitude from the live channel id. This naming is confusing - consider renaming to mirror_id or remap_target_id to clarify that this is the target channel receiving the remapped amplitude.

Copilot uses AI. Check for mistakes.
registry.fill(HIST("FT0Amp"), dead_id, ampl);
ampl = ampl / cstFT0RelGain[iCh];
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect array indexing: cstFT0RelGain[iCh] uses the loop index iCh, but cstFT0RelGain is indexed by channel ID (0-207), not by the position in the sparse channelC() array. This should be cstFT0RelGain[dead_id] since dead_id is the actual channel ID.

Copilot uses AI. Check for mistakes.
registry.fill(HIST("FT0AmpCorrect"), dead_id, ampl);
}
}
if ((cfgRejectFT0CInside && (id >= kFT0CInnerRingMin && id <= kFT0CInnerRingMax)) || (cfgRejectFT0COutside && (id >= kFT0COuterRingMin && id <= kFT0COuterRingMax)))
ampl = 0.;
registry.fill(HIST("FT0Amp"), id, ampl);
Expand All @@ -661,6 +689,14 @@ struct LongRangeDihadronCor {
} else if (fitType == kFT0A) {
id = ft0.channelA()[iCh];
ampl = ft0.amplitudeA()[iCh];
if (cfgRemapFT0ADeadChannels) {
if (id >= kFT0ARemapChannelStart && id <= kFT0ARemapChannelEnd) {
int dead_id = id - kFT0AOuterMirror;
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name dead_id suggests this represents a dead channel ID that should receive data, but the implementation fills histogram data for dead_id using amplitude from the live channel id. This naming is confusing - consider renaming to mirror_id or remap_target_id to clarify that this is the target channel receiving the remapped amplitude.

Copilot uses AI. Check for mistakes.
registry.fill(HIST("FT0Amp"), dead_id, ampl);
ampl = ampl / cstFT0RelGain[iCh];
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect array indexing: cstFT0RelGain[iCh] uses the loop index iCh, but cstFT0RelGain is indexed by channel ID (0-207), not by the position in the sparse channelA() array. This should be cstFT0RelGain[dead_id] since dead_id is the actual channel ID.

Copilot uses AI. Check for mistakes.
registry.fill(HIST("FT0AmpCorrect"), dead_id, ampl);
}
}
if ((cfgRejectFT0AInside && (id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax)) || (cfgRejectFT0AOutside && (id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax)))
ampl = 0.;
registry.fill(HIST("FT0Amp"), id, ampl);
Expand Down
Loading