Conversation
There was a problem hiding this comment.
Pull request overview
Updates the LongRangeDihadronCor analysis task to support FT0 dead-channel “mirroring” behavior and optional run-by-run FT0 amplitude QA output, while reorganizing forward-detector configurables and adjusting PID n-sigma configuration labels.
Changes:
- Introduces a forward-detector configurable group (
cfgFwdConfig) including FT0 channel rejection, dead-channel mirroring toggles, and a run-by-run FT0 amplitude QA option. - Adds per-run histogram creation/tracking (
histAmpCorrectPerRun) and fills it during same-event processing. - Updates the PID
nSigmaslabeled array layout/labels (now “upCut/lowCut” style).
💡 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 the per-run histogram already exists, but still proceeds to add another histogram with the same name and then insert into the map. This can create duplicate registry objects and the insert will not replace the existing entry. Consider returning early (or skipping creation) when histAmpCorrectPerRun already contains runNumber.
| 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 labeled array column labels were changed to upCut_/lowCut_ but the help text still says the values are for “positive and negative” particles. Please update the description to match the new semantics (upper/lower cuts), otherwise the configuration becomes misleading for users.
| 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 cuts in TPC, TOF, ITS for pions, kaons, protons (upCut_*/lowCut_*)"}; |
| 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.
Minor: histAmpCorrectPerRun is a map of TH2 histograms, but the trailing comment says “map of TH3 histograms”. Please fix the comment to match the actual type.
| 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 (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.
In getChannel(), histAmpCorrectPerRun[lastRunNumber]->Fill(...) is executed for SameEvent without checking cfgFwdConfig.cfgRunbyRunAmplitudeFT0 (or that the map contains lastRunNumber). If the run-by-run option is disabled, histAmpCorrectPerRun is empty and this will create a default null shared_ptr via operator[] and then dereference it, causing a crash. Please guard the fill with cfgFwdConfig.cfgRunbyRunAmplitudeFT0 and a presence check (or use at()/find() and only Fill when the histogram exists).
No description provided.