Conversation
There was a problem hiding this comment.
Pull request overview
Updates the long-range di-hadron correlation task to extend FT0 handling (dead-channel mirroring and optional run-by-run amplitude QA) and to reorganize forward-detector configurables.
Changes:
- Simplifies the PID
nSigmaslabeled-array layout and renames its labels to upper/lower cut semantics. - Moves FT0 rejection/remapping-related configurables into a dedicated
cfgFwdConfiggroup and introduces dead-channel “mirroring” options. - Adds optional run-by-run FT0 amplitude-correction histograms and fills them during SameEvent processing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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 a histogram for the run already exists, but still calls registry.add() again. Even though map::insert won’t replace the existing entry, registry.add() may create a duplicate object/path (or fail) in the output. Return early when the run already exists, or use try_emplace and only create the histogram when insertion succeeds.
| if (cfgDrawEtaPhiDis && system == SameEvent) { | ||
| registry.fill(HIST("EtaPhi"), eta, phi, ampl * eventWeight); | ||
| if (mirrorChannel) | ||
| registry.fill(HIST("EtaPhi"), eta, 4 * PIHalf - phi, ampl * eventWeight); | ||
| } | ||
| float deltaPhi = RecoDecay::constrainAngle(track1.phi() - phi, -PIHalf); | ||
| float deltaEta = track1.eta() - eta; | ||
| // fill the right sparse and histograms | ||
| if (system == SameEvent) { | ||
| if (corType == kFT0A) { | ||
| registry.fill(HIST("Assoc_amp_same_TPC_FT0A"), chanelid, ampl); | ||
| sameTpcFt0a->getPairHist()->Fill(step, fSampleIndex, posZ, track1.pt(), track1.pt(), deltaPhi, deltaEta, ampl * eventWeight * triggerWeight); | ||
| registry.fill(HIST("deltaEta_deltaPhi_same_TPC_FT0A"), deltaPhi, deltaEta, ampl * eventWeight * triggerWeight); | ||
| sameTpcFt0a->getPairHist()->Fill(step, fSampleIndex, posZ, track1.pt(), track1.pt(), deltaPhi, deltaEta, ampl * eventWeight * triggerWeight); | ||
| if (mirrorChannel) | ||
| sameTpcFt0a->getPairHist()->Fill(step, fSampleIndex, posZ, track1.pt(), track1.pt(), RecoDecay::constrainAngle(track1.phi() - phi - 2 * PIHalf, -PIHalf), deltaEta, ampl * eventWeight * triggerWeight); |
There was a problem hiding this comment.
Mirroring uses inconsistent phi transformations: the QA histogram uses phi_mirror = 4*PIHalf - phi (i.e. 2π−phi), but the correlation fill uses track1.phi() - phi - 2*PIHalf (a π shift). This will make mirrored QA and mirrored correlations disagree. Define a single helper to compute the mirrored FT0 phi (or mirrored deltaPhi) and use it consistently in both places.
| 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.
When both mirrorChannelA and mirrorChannelC are true, this code fills the pair histogram a second time with the exact same (deltaPhi, deltaEta, weight) that was already filled unconditionally above, causing double counting for those channel pairs. Remove the redundant fill, or replace it with the intended “mirror both sides” deltaPhi calculation.
| 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.
New code uses std::map, std::shared_ptr, and std::find but this file doesn’t include <map>, <memory>, or <algorithm>. Relying on transitive includes is brittle and can break builds; please add the missing standard headers explicitly.
| 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 help text for nSigmas still says the 6 entries are for “positive and negative”, but the labels were changed to upper/lower cut values (upCut_* / lowCut_*). Please update the description string to match the new semantics so configuration is self-explanatory.
| 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 n-sigma upper/lower cut values for TPC, TOF, ITS for pions, kaons, protons (columns: upCut_pi, upCut_ka, upCut_pr, lowCut_pi, lowCut_ka, lowCut_pr)"}; |
| int currentRunNumber = bc.runNumber(); | ||
| if (cfgFwdConfig.cfgRunbyRunAmplitudeFT0 && currentRunNumber != lastRunNumber) { | ||
| lastRunNumber = currentRunNumber; | ||
| if (std::find(runNumbers.begin(), runNumbers.end(), currentRunNumber) == runNumbers.end()) { | ||
| // if run number is not in the preconfigured list, create new output histograms for this run | ||
| createOutputObjectsForRun(currentRunNumber); | ||
| runNumbers.push_back(currentRunNumber); | ||
| LOGF(info, "Created Run-by-run objects in processSameTpcFt0a"); | ||
| } | ||
| if (histAmpCorrectPerRun.find(currentRunNumber) == histAmpCorrectPerRun.end()) { | ||
| LOGF(fatal, "RunNumber %d not found in histAmpCorrectPerRun", currentRunNumber); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The run-by-run initialization block is duplicated across multiple processSame* functions. Consider extracting it into a small helper (e.g. ensureRunHistograms(bc.runNumber(), "processSameTpcFt0a")) to reduce repetition and the chance of these paths diverging over time.
| int currentRunNumber = bc.runNumber(); | |
| if (cfgFwdConfig.cfgRunbyRunAmplitudeFT0 && currentRunNumber != lastRunNumber) { | |
| lastRunNumber = currentRunNumber; | |
| if (std::find(runNumbers.begin(), runNumbers.end(), currentRunNumber) == runNumbers.end()) { | |
| // if run number is not in the preconfigured list, create new output histograms for this run | |
| createOutputObjectsForRun(currentRunNumber); | |
| runNumbers.push_back(currentRunNumber); | |
| LOGF(info, "Created Run-by-run objects in processSameTpcFt0a"); | |
| } | |
| if (histAmpCorrectPerRun.find(currentRunNumber) == histAmpCorrectPerRun.end()) { | |
| LOGF(fatal, "RunNumber %d not found in histAmpCorrectPerRun", currentRunNumber); | |
| return; | |
| } | |
| auto ensureRunHistograms = [&](int currentRunNumber, const char* caller) -> bool { | |
| if (cfgFwdConfig.cfgRunbyRunAmplitudeFT0 && currentRunNumber != lastRunNumber) { | |
| lastRunNumber = currentRunNumber; | |
| if (std::find(runNumbers.begin(), runNumbers.end(), currentRunNumber) == runNumbers.end()) { | |
| // if run number is not in the preconfigured list, create new output histograms for this run | |
| createOutputObjectsForRun(currentRunNumber); | |
| runNumbers.push_back(currentRunNumber); | |
| LOGF(info, "Created Run-by-run objects in %s", caller); | |
| } | |
| if (histAmpCorrectPerRun.find(currentRunNumber) == histAmpCorrectPerRun.end()) { | |
| LOGF(fatal, "RunNumber %d not found in histAmpCorrectPerRun", currentRunNumber); | |
| return false; | |
| } | |
| } | |
| return true; | |
| }; | |
| int currentRunNumber = bc.runNumber(); | |
| if (!ensureRunHistograms(currentRunNumber, "processSameTpcFt0a")) { | |
| return; |
| 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.
getChannel() unconditionally dereferences histAmpCorrectPerRun[lastRunNumber] in SameEvent. When cfgFwdConfig.cfgRunbyRunAmplitudeFT0 is false (default) or no histogram was created for lastRunNumber, operator[] will insert a null shared_ptr and ->Fill() will crash. Gate this fill behind cfgRunbyRunAmplitudeFT0 and use find()/at() with a null check (or pass the current run histogram pointer into getChannel).
| 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.
Same issue as above for FT0A branch: histAmpCorrectPerRun[lastRunNumber]->Fill(...) can dereference a null shared_ptr when run-by-run output is disabled or not initialized for lastRunNumber. Please guard this fill or look up the histogram safely before calling Fill().
1 participant
No description provided.