Skip to content

Conversation

@MartijnLaarhoven
Copy link
Owner

No description provided.

Copilot AI review requested due to automatic review settings February 12, 2026 13:27
@github-actions github-actions bot added the pwgcf label Feb 12, 2026
@github-actions github-actions bot changed the title Add files via upload [PWGCF] Add files via upload Feb 12, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the long-range dihadron correlation task to support additional forward-detector (FT0) handling, including “mirroring” logic for selected channels and optional run-by-run FT0 amplitude QA output.

Changes:

  • Refactors FT0 forward-related configurables into a dedicated cfgFwdConfig group and renames “remap dead channels” to “mirror dead channels”.
  • Adds optional run-by-run FT0 amplitude-corrected histograms keyed by run number.
  • Updates FT0 correlation filling to optionally add mirrored contributions and adjusts PID nSigmas labeled array defaults/labels.

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

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(cfgMirrorFT0ADeadChannels, bool, false, "If true, mirror FT0A channels 60-63 to amplitudes from 92-95 respectively")
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The help text for cfgMirrorFT0ADeadChannels says it mirrors channels 60–63 using amplitudes from 92–95, but the implementation only flags IDs in 92–95 (and never fills/labels 60–63 anywhere, e.g. Assoc_amp_* is still filled with the original channel ID). Either update the help text to match the implemented behavior, or implement the actual remap/mirroring so that 60–63 are populated as described.

Suggested change
O2_DEFINE_CONFIGURABLE(cfgMirrorFT0ADeadChannels, bool, false, "If true, mirror FT0A channels 60-63 to amplitudes from 92-95 respectively")
O2_DEFINE_CONFIGURABLE(cfgMirrorFT0ADeadChannels, bool, false, "If true, use FT0A channels 92-95 as mirrors for dead channels 60-63 (channel IDs remain 92-95)")

Copilot uses AI. Check for mistakes.
std::array<float, 6> tpcNsigmaCut;
int lastRunNumber = -1;
std::vector<int> runNumbers;
std::map<int, std::shared_ptr<TH2>> histAmpCorrectPerRun; // map of TH3 histograms for all runs
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

Comment mismatch: histAmpCorrectPerRun is declared as a map of TH2, but the trailing comment says "map of TH3 histograms". Please update the comment to avoid confusion when maintaining the run-by-run QA logic.

Suggested change
std::map<int, std::shared_ptr<TH2>> histAmpCorrectPerRun; // map of TH3 histograms for all runs
std::map<int, std::shared_ptr<TH2>> histAmpCorrectPerRun; // map of TH2 histograms for all runs

Copilot uses AI. Check for mistakes.
Comment on lines +57 to 61
#include <map>
#include <memory>
#include <string>
#include <utility>
#include <vector>
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

std::find is used later (e.g., in processSame* run-by-run logic), but <algorithm> is not included. This will fail to compile on standard-compliant toolchains; add #include <algorithm> in the include block.

Copilot uses AI. Check for mistakes.
Comment on lines +689 to +707
if (system == SameEvent)
registry.fill(HIST("FT0Amp"), id, ampl);
ampl = ampl / cstFT0RelGain[iCh];
registry.fill(HIST("FT0AmpCorrect"), id, ampl);
if (system == SameEvent) {
registry.fill(HIST("FT0AmpCorrect"), id, ampl);
histAmpCorrectPerRun[lastRunNumber]->Fill(id, ampl);
}
} else if (fitType == kFT0A) {
id = ft0.channelA()[iCh];
ampl = ft0.amplitudeA()[iCh];
if (cfgRemapFT0ADeadChannels) {
if (id >= kFT0ARemapChannelStart && id <= kFT0ARemapChannelEnd) {
int dead_id = id - kFT0AOuterMirror;
float mirroredAmpl = ampl;
float mirroredAmplCorrected = mirroredAmpl / cstFT0RelGain[iCh];
registry.fill(HIST("FT0Amp"), dead_id, mirroredAmpl);
registry.fill(HIST("FT0AmpCorrect"), dead_id, mirroredAmplCorrected);
}
}
if ((cfgRejectFT0AInside && (id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax)) || (cfgRejectFT0AOutside && (id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax)))
if ((cfgFwdConfig.cfgRejectFT0AInside && (id >= kFT0AInnerRingMin && id <= kFT0AInnerRingMax)) || (cfgFwdConfig.cfgRejectFT0AOutside && (id >= kFT0AOuterRingMin && id <= kFT0AOuterRingMax)))
ampl = 0.;
registry.fill(HIST("FT0Amp"), id, ampl);
if (system == SameEvent)
registry.fill(HIST("FT0Amp"), id, ampl);
ampl = ampl / cstFT0RelGain[iCh];
registry.fill(HIST("FT0AmpCorrect"), id, ampl);
if (system == SameEvent) {
registry.fill(HIST("FT0AmpCorrect"), id, ampl);
histAmpCorrectPerRun[lastRunNumber]->Fill(id, ampl);
}
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

getChannel() unconditionally calls histAmpCorrectPerRun[lastRunNumber]->Fill(...) for SameEvent. When cfgRunbyRunAmplitudeFT0 is false (or before a run histogram is created), operator[] will insert a null shared_ptr and this will dereference a null pointer. Guard this fill behind cfgFwdConfig.cfgRunbyRunAmplitudeFT0 and a successful map lookup (or pass the per-run histogram pointer explicitly).

Copilot uses AI. Check for mistakes.
Comment on lines +787 to +788
if (mirrorChannel)
registry.fill(HIST("EtaPhi"), eta, 4 * PIHalf - phi, ampl * eventWeight);
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The mirrored EtaPhi fill uses 4 * PIHalf - phi (i.e., 2π - phi), but the mirrored correlation fills treat mirroring as a rotation (see the - 2 * PIHalf delta-phi shift). These two transformations are inconsistent; use the same phi mapping for both (typically phi' = phi + π wrapped to [0, 2π)) so QA and correlation content match.

Suggested change
if (mirrorChannel)
registry.fill(HIST("EtaPhi"), eta, 4 * PIHalf - phi, ampl * eventWeight);
if (mirrorChannel) {
auto phiMirror = phi + 2 * PIHalf; // mirror as +π rotation
if (phiMirror >= 4 * PIHalf) {
phiMirror -= 4 * PIHalf; // wrap to [0, 2π)
}
registry.fill(HIST("EtaPhi"), eta, phiMirror, ampl * eventWeight);
}

Copilot uses AI. Check for mistakes.
Comment on lines +868 to +874
if (mirrorChannelA) {
sameFt0aFt0c->getPairHist()->Fill(step, fSampleIndex, posZ, 0.5, 0.5, RecoDecay::constrainAngle(phiA + 2 * PIHalf - phiC, -PIHalf), deltaEta, amplA * amplC * eventWeight * triggerWeight);
if (mirrorChannelC)
sameFt0aFt0c->getPairHist()->Fill(step, fSampleIndex, posZ, 0.5, 0.5, deltaPhi, deltaEta, amplA * amplC * eventWeight * triggerWeight);
}
if (mirrorChannelC)
sameFt0aFt0c->getPairHist()->Fill(step, fSampleIndex, posZ, 0.5, 0.5, RecoDecay::constrainAngle(phiA - phiC - 2 * PIHalf, -PIHalf), deltaEta, amplA * amplC * eventWeight * triggerWeight);
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

When both mirrorChannelA and mirrorChannelC are true, the code fills the pair histogram again with deltaPhi unchanged, which duplicates the nominal entry and biases the distribution. If you intended to add only the rotated combinations, remove this duplicate fill or replace it with the correctly transformed (A', C') combination (which may already be covered by other branches).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant