[PWGCF] Adding a task for short range correlations#14670
[PWGCF] Adding a task for short range correlations#14670victor-gonzalez merged 2 commits intoAliceO2Group:masterfrom
Conversation
Adding a task for short range correlations as a function of eta
[PWGCF] Adding a task for short range correlations
There was a problem hiding this comment.
Pull request overview
This pull request adds a new di-hadron correlation analysis task that measures short-range correlations as a function of pseudorapidity (η), alongside wiring it into the analysis workflows.
Changes:
- Introduces
EtaDihadronanalysis task implementing η-differential same- and mixed-event di-hadron correlations, including MC and efficiency handling, largely mirroring the existingDiHadronCortask but using η for trigger/associate axes. - Defines new configurable η-binned axes for trigger and associated particles and updates all relevant histograms and correlation containers to use these.
- Registers a new DPL workflow
eta-dihadroninCMakeLists.txtso the task can be run within the PWGCF TwoParticleCorrelations framework.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
PWGCF/TwoParticleCorrelations/Tasks/etaDihadron.cxx |
New η-differential di-hadron correlation task, including configuration, event/track selection, efficiency and centrality weighting, same/mixed-event correlation filling, and corresponding MC/efficiency workflows. |
PWGCF/TwoParticleCorrelations/Tasks/CMakeLists.txt |
Adds the eta-dihadron workflow target referencing etaDihadron.cxx and linking it with the standard PWGCF analysis libraries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (doprocessSame && cfgUseAdditionalEventCut) { | ||
| registry.add("hEventCountSpecific", "Number of Event;; Count", {HistType::kTH1D, {{12, 0, 12}}}); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(1, "after sel8"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(2, "kNoSameBunchPileup"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(3, "kNoITSROFrameBorder"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(4, "kNoTimeFrameBorder"); |
There was a problem hiding this comment.
hEventCountSpecific is created only when doprocessSame && cfgUseAdditionalEventCut is true, but eventSelected() fills this histogram unconditionally whenever cfgUseAdditionalEventCut is enabled (including when it is called from processMixed). This means that configurations where only the mixed-event processing is enabled with additional event cuts will call registry.fill(HIST("hEventCountSpecific"), ...) on a histogram that was never registered, which can lead to a runtime error. Consider removing the doprocessSame condition when registering hEventCountSpecific, or alternatively guarding all fills in eventSelected() behind the same condition that controls the histogram’s creation.
There was a problem hiding this comment.
I would follow Copilot recommendation
If not, why is being used?
|
@vkucera has the linter been removed from CI? I don't see it neither in the checks nor its report |
| registry.add("hEventCountSpecific", "Number of Event;; Count", {HistType::kTH1D, {{12, 0, 12}}}); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(1, "after sel8"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(2, "kNoSameBunchPileup"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(3, "kNoITSROFrameBorder"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(4, "kNoTimeFrameBorder"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(5, "kIsGoodZvtxFT0vsPV"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(6, "kNoCollInTimeRangeStandard"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(7, "kIsGoodITSLayersAll"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(8, "kNoCollInRofStandard"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(9, "kNoHighMultCollInPrevRof"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(10, "occupancy"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(11, "MultCorrelation"); | ||
| registry.get<TH1>(HIST("hEventCountSpecific"))->GetXaxis()->SetBinLabel(12, "cfgEvSelV0AT0ACut"); |
There was a problem hiding this comment.
Avoid using magic numbers
If an enum is defined with the corresponding named constants, the x axis is started in -0.5, and the labels set to the named constant plus one, the named constants can be straight on used when filling the histogram which is less error prone, provides readability and easy expansion in the future
| template <typename TCollision> | ||
| bool eventSelected(TCollision collision, const int multTrk, const float centrality, const bool fillCounter) | ||
| { | ||
| registry.fill(HIST("hEventCountSpecific"), 0.5); |
There was a problem hiding this comment.
See my previous comment on the histogram definition
| } | ||
| PROCESS_SWITCH(EtaDihadron, processMCMixed, "Process MC mixed events", false); | ||
|
|
||
| void processOntheflySame(aod::McCollisions::iterator const& mcCollision, aod::McParticles const& mcParticles) |
There was a problem hiding this comment.
Would not be more readable processOnTheFlySame?
| } | ||
| PROCESS_SWITCH(EtaDihadron, processOntheflySame, "Process on-the-fly same event", false); | ||
|
|
||
| void processOntheflyMixed(aod::McCollisions const& mcCollisions, aod::McParticles const& mcParticles) |
There was a problem hiding this comment.
Same but the equivalent
victor-gonzalez
left a comment
There was a problem hiding this comment.
Please, have a look at my comments for future iterations
Adding a task for short range correlations as a function of eta