Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the long-range dihadron correlation task to extend FT0 forward-detector handling, including new configuration grouping, optional run-by-run FT0 amplitude QA, and revised “mirroring” logic for selected FT0 channels.
Changes:
- Add a forward-detector configurable group (
cfgFwdConfig) and introduce optional run-by-run FT0 amplitude output histograms. - Replace/remodel FT0 dead-channel handling via “mirroring” and update how FT0 amplitudes are filled for SameEvent vs MixedEvent.
- Clean up the PID n-sigma labeled-array defaults/labels and trim unused initializer data.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| O2_DEFINE_CONFIGURABLE(cfgPIDParticle, int, 0, "1 = pion, 2 = kaon, 3 = proton, 4 = kshort, 5 = lambda, 6 = phi, 0 for no PID") | ||
| O2_DEFINE_CONFIGURABLE(cfgTofPtCut, float, 0.5f, "Minimum pt to use TOF N-sigma") | ||
| Configurable<LabeledArray<float>> nSigmas{"nSigmas", {LongArrayFloat[0], 3, 6, {"TPC", "TOF", "ITS"}, {"pos_pi", "pos_ka", "pos_pr", "neg_pi", "neg_ka", "neg_pr"}}, "Labeled array for n-sigma values for TPC, TOF, ITS for pions, kaons, protons (positive and negative)"}; | ||
| Configurable<LabeledArray<float>> nSigmas{"nSigmas", {LongArrayFloat[0], 3, 6, {"TPC", "TOF", "ITS"}, {"upCut_pi", "upCut_ka", "upCut_pr", "lowCut_pi", "lowCut_ka", "lowCut_pr"}}, "Labeled array for n-sigma values for TPC, TOF, ITS for pions, kaons, protons (positive and negative)"}; |
There was a problem hiding this comment.
The nSigmas labels were changed to upCut_*/lowCut_*, but the help string still says “positive and negative”. Please update the description to match the actual meaning (upper/lower n-sigma cuts).
| Configurable<LabeledArray<float>> nSigmas{"nSigmas", {LongArrayFloat[0], 3, 6, {"TPC", "TOF", "ITS"}, {"upCut_pi", "upCut_ka", "upCut_pr", "lowCut_pi", "lowCut_ka", "lowCut_pr"}}, "Labeled array for n-sigma values for TPC, TOF, ITS for pions, kaons, protons (positive and negative)"}; | |
| Configurable<LabeledArray<float>> nSigmas{"nSigmas", {LongArrayFloat[0], 3, 6, {"TPC", "TOF", "ITS"}, {"upCut_pi", "upCut_ka", "upCut_pr", "lowCut_pi", "lowCut_ka", "lowCut_pr"}}, "Labeled array for upper and lower n-sigma cuts in TPC, TOF, ITS for pions, kaons, protons"}; |
| 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") | ||
| O2_DEFINE_CONFIGURABLE(cfgMirrorFT0CDeadChannels, bool, false, "If true, mirror FT0C channels 177->145, 176->144, 178->146, 179->147, 139->115") |
There was a problem hiding this comment.
The help text for cfgMirrorFT0CDeadChannels describes mirroring 177->145, 176->144, ... , 139->115, but isMirrorId() currently flags IDs 115 and 144–147 as the ones to mirror. Please align the help text and the implemented ID selection so it’s unambiguous which channels are treated as sources vs destinations.
| O2_DEFINE_CONFIGURABLE(cfgMirrorFT0CDeadChannels, bool, false, "If true, mirror FT0C channels 177->145, 176->144, 178->146, 179->147, 139->115") | |
| O2_DEFINE_CONFIGURABLE(cfgMirrorFT0CDeadChannels, bool, false, "If true, mirror FT0C channels 115, 144, 145, 146, 147 to amplitudes from 139, 176, 177, 178, 179 respectively") |
| 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 |
There was a problem hiding this comment.
Comment says “map of TH3 histograms”, but the type is std::map<int, std::shared_ptr<TH2>>. Please correct the comment to avoid confusion.
| 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 |
| if (cfgFwdConfig.cfgRunbyRunAmplitudeFT0) { | ||
| if (histAmpCorrectPerRun.find(runNumber) != histAmpCorrectPerRun.end()) { | ||
| LOGF(info, "you are trying to create QA hist again, please make sure you are not filling it twice"); | ||
| } | ||
| const AxisSpec axisFit{1000, 0, 5000, "FIT amplitude"}; | ||
| const AxisSpec axisChID{220, 0, 220, "FIT channel"}; | ||
| std::shared_ptr<TH2> histFT0AmpCorrect = registry.add<TH2>(Form("%d/FT0AmpCorrect", runNumber), "FIT channel;FIT amplitude", {HistType::kTH2F, {axisChID, axisFit}}); | ||
| histAmpCorrectPerRun.insert(std::make_pair(runNumber, histFT0AmpCorrect)); | ||
| } |
There was a problem hiding this comment.
createOutputObjectsForRun() logs when the run histogram already exists, but then continues to call registry.add() and insert(). This can lead to duplicate registry objects and/or the map not being updated (since insert won’t overwrite). Consider returning early when the key exists, or using try_emplace/overwrite semantics explicitly.
| 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); |
There was a problem hiding this comment.
histAmpCorrectPerRun[lastRunNumber]->Fill(...) is executed for every SameEvent call, but it isn’t guarded by cfgFwdConfig.cfgRunbyRunAmplitudeFT0 and uses operator[], which will insert a null entry when the key doesn’t exist (e.g. when the feature is disabled or before per-run objects are created), causing a crash. Please guard this fill behind the config flag and a find() check (or pass the current run number and use at() after ensuring creation).
| if (mirrorChannel) | ||
| registry.fill(HIST("EtaPhi"), eta, 4 * PIHalf - phi, ampl * eventWeight); |
There was a problem hiding this comment.
For mirrored channels, EtaPhi is filled at 4*PIHalf - phi (i.e. 2π - φ), but the correlation mirroring uses a π rotation (track.phi() - phi - 2*PIHalf). These are different transformations (reflection vs 180° rotation). Please use the same transformation for the QA EtaPhi fill as the one used for the correlation (or document why they should differ).
| if (mirrorChannel) | |
| registry.fill(HIST("EtaPhi"), eta, 4 * PIHalf - phi, ampl * eventWeight); | |
| if (mirrorChannel) { | |
| float phiMirror = phi + 2 * PIHalf; // π rotation, consistent with correlation mirroring | |
| if (phiMirror >= 4 * PIHalf) { | |
| phiMirror -= 4 * PIHalf; // wrap into [0, 2π) | |
| } | |
| registry.fill(HIST("EtaPhi"), eta, phiMirror, ampl * eventWeight); | |
| } |
| sameFt0aFt0c->getPairHist()->Fill(step, fSampleIndex, posZ, 0.5, 0.5, deltaPhi, deltaEta, amplA * amplC * eventWeight * triggerWeight); | ||
| 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); | ||
| } |
There was a problem hiding this comment.
Inside the mirrorChannelA block, the nested if (mirrorChannelC) fills the pair histogram again using the exact same deltaPhi/deltaEta as the nominal fill above. If this is meant to represent the (A′, C′) mirrored combination, consider making that intent explicit (e.g. compute using mirrored angles) or add a clarifying comment; otherwise this will look like accidental double counting.
|
|
||
| #include <map> | ||
| #include <memory> | ||
| #include <utility> |
There was a problem hiding this comment.
std::find is used later in this file, but <algorithm> isn’t included. Please add #include <algorithm> to avoid relying on transitive includes.
| #include <utility> | |
| #include <utility> | |
| #include <algorithm> |
1 participant
No description provided.