Skip to content

[PWGCF] Adding an option to reject part of the FT0 detectors #14242

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 an option to reject part of the FT0 detectors #14242

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e63b492
Add files via upload
MartijnLaarhoven Dec 12, 2025
5c3b91b
Add files via upload
MartijnLaarhoven Dec 12, 2025
7b918e4
Update PWGCF/TwoParticleCorrelations/Tasks/longRangeDihadronCor.cxx
MartijnLaarhoven Dec 12, 2025
d691be8
Merge branch 'AliceO2Group:master' into master
MartijnLaarhoven Dec 15, 2025
24189d9
Merge branch 'AliceO2Group:master' into master
MartijnLaarhoven Dec 15, 2025
3468fb2
Add files via upload
MartijnLaarhoven Dec 15, 2025
3a692c9
Apply suggestions from code review
MartijnLaarhoven Dec 16, 2025
d6fd502
Add files via upload
MartijnLaarhoven Dec 16, 2025
ee2a012
Merge branch 'AliceO2Group:master' into master
MartijnLaarhoven Dec 16, 2025
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
18 changes: 18 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 @@ -107,6 +107,10 @@ struct LongRangeDihadronCor {
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 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")
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

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).

Suggested change
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")

Copilot uses AI. Check for mistakes.
O2_DEFINE_CONFIGURABLE(cfgRejectFT0COutside, bool, false, "Rejection of outside ring channels of the FT0C detector")
Comment on lines +110 to +113
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
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")

Copilot uses AI. Check for mistakes.
Comment on lines +111 to +113
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

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).

Suggested change
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")

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

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).

Suggested change
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")

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 @@ -236,6 +240,16 @@ struct LongRangeDihadronCor {
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;
Expand Down Expand Up @@ -639,12 +653,16 @@ struct LongRangeDihadronCor {
id = ft0.channelC()[iCh];
id = id + Ft0IndexA;
ampl = ft0.amplitudeC()[iCh];
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);
Comment on lines +656 to 660
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
} else if (fitType == kFT0A) {
id = ft0.channelA()[iCh];
ampl = ft0.amplitudeA()[iCh];
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);
Comment on lines +664 to 668
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Expand Down
Loading