Skip to content

[PWGCF] Adding a task for short range correlations#8

Closed
MartijnLaarhoven wants to merge 1 commit intomasterfrom
MartijnLaarhoven-patch-6
Closed

[PWGCF] Adding a task for short range correlations#8
MartijnLaarhoven wants to merge 1 commit intomasterfrom
MartijnLaarhoven-patch-6

Conversation

@MartijnLaarhoven
Copy link
Owner

Adding a task for short range correlations as a function of eta

Copilot AI review requested due to automatic review settings January 28, 2026 12:13
@github-actions github-actions bot added the pwgcf label Jan 28, 2026
@github-actions github-actions bot changed the title Adding a task for short range correlations [PWGCF] Adding a task for short range correlations Jan 28, 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

Adds a new analysis task to compute short-range di-hadron correlations as a function of pseudorapidity (η), and registers it as a runnable DPL workflow.

Changes:

  • Introduces EtaDihadron task implementing same-/mixed-event η-dependent di-hadron correlations with event/track selection and optional corrections.
  • Adds a new DPL workflow target eta-dihadron to the Tasks CMake configuration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
PWGCF/TwoParticleCorrelations/Tasks/etaDihadron.cxx New η-differential di-hadron correlation task (same/mixed event, selection, corrections, histograms).
PWGCF/TwoParticleCorrelations/Tasks/CMakeLists.txt Registers the new eta-dihadron workflow build target.

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

if (!getEfficiencyCorrection(triggerWeight, track1.eta(), track1.pt(), posZ))
continue;
if (system == SameEvent) {
registry.fill(HIST("Trig_hist"), fSampleIndex, posZ, track1.pt(), eventWeight * triggerWeight);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Trig_hist is filled with track1.pt() but the histogram is defined with axisEtaTrigger. This mismatch will distort/empty the histogram. Update the filled quantity (eta) or change the histogram axis to a pT axis.

Suggested change
registry.fill(HIST("Trig_hist"), fSampleIndex, posZ, track1.pt(), eventWeight * triggerWeight);
registry.fill(HIST("Trig_hist"), fSampleIndex, posZ, track1.eta(), eventWeight * triggerWeight);

Copilot uses AI. Check for mistakes.
template <typename TCollision>
bool eventSelected(TCollision collision, const int multTrk, const float centrality, const bool fillCounter)
{
registry.fill(HIST("hEventCountSpecific"), 0.5);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

eventSelected() unconditionally fills hEventCountSpecific, but that histogram is only added in init() when doprocessSame && cfgUseAdditionalEventCut. If the user disables processSame but keeps processMixed enabled (and cfgUseAdditionalEventCut=true), this will attempt to fill a non-existent histogram and can crash. Either always register hEventCountSpecific when cfgUseAdditionalEventCut is enabled, or guard the registry.fill() calls based on histogram existence / fillCounter.

Suggested change
registry.fill(HIST("hEventCountSpecific"), 0.5);
if (fillCounter) {
registry.fill(HIST("hEventCountSpecific"), 0.5);
}

Copilot uses AI. Check for mistakes.
Comment on lines +426 to +428
LOGF(fatal, "Could not load efficiency histogram for trigger particles from %s", cfgCentralityWeight.value.c_str());
}
LOGF(info, "Loaded efficiency histogram from %s (%p)", cfgCentralityWeight.value.c_str(), (void*)mCentralityWeight);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The fatal error message for failing to load cfgCentralityWeight says "efficiency histogram for trigger particles". This is misleading while debugging configuration problems; adjust the message to mention the centrality weight histogram/path.

Suggested change
LOGF(fatal, "Could not load efficiency histogram for trigger particles from %s", cfgCentralityWeight.value.c_str());
}
LOGF(info, "Loaded efficiency histogram from %s (%p)", cfgCentralityWeight.value.c_str(), (void*)mCentralityWeight);
LOGF(fatal, "Could not load centrality weight histogram from path %s (cfgCentralityWeight)", cfgCentralityWeight.value.c_str());
}
LOGF(info, "Loaded centrality weight histogram from %s (%p)", cfgCentralityWeight.value.c_str(), (void*)mCentralityWeight);

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +102
O2_DEFINE_CONFIGURABLE(cfgUsePtOrder, bool, true, "enable trigger pT < associated pT cut")
O2_DEFINE_CONFIGURABLE(cfgUsePtOrderInMixEvent, bool, true, "enable trigger pT < associated pT cut in mixed event")
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The cfgUsePtOrder / cfgUsePtOrderInMixEvent help strings say "trigger pT < associated pT", but the actual selection logic keeps only pairs with trigger pT > associated pT (continue when track1.pt() <= track2.pt()). Update the help text to match the implemented behavior so users configure it correctly.

Suggested change
O2_DEFINE_CONFIGURABLE(cfgUsePtOrder, bool, true, "enable trigger pT < associated pT cut")
O2_DEFINE_CONFIGURABLE(cfgUsePtOrderInMixEvent, bool, true, "enable trigger pT < associated pT cut in mixed event")
O2_DEFINE_CONFIGURABLE(cfgUsePtOrder, bool, true, "enable trigger pT > associated pT cut")
O2_DEFINE_CONFIGURABLE(cfgUsePtOrderInMixEvent, bool, true, "enable trigger pT > associated pT cut in mixed event")

Copilot uses AI. Check for mistakes.
registry.add("Centrality_used", hCentTitle.c_str(), {HistType::kTH1D, {{100, 0, 100}}}); // histogram to see how many events are in the same and mixed event
registry.add("zVtx", "zVtx", {HistType::kTH1D, {axisVertex}});
registry.add("zVtx_used", "zVtx_used", {HistType::kTH1D, {axisVertex}});
registry.add("Trig_hist", "", {HistType::kTHnSparseF, {{axisSample, axisVertex, axisEtaTrigger}}});
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Trig_hist is defined with axisEtaTrigger (eta binning), but later it is filled with track1.pt(). This will push essentially all entries into overflow/underflow and make the histogram incorrect. Make the axis and the filled variable consistent (either use an axisPt-based axis here, or fill with track1.eta() if the intent is an eta distribution).

Suggested change
registry.add("Trig_hist", "", {HistType::kTHnSparseF, {{axisSample, axisVertex, axisEtaTrigger}}});
registry.add("Trig_hist", "", {HistType::kTHnSparseF, {{axisSample, axisVertex, axisPt}}});

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

Is it not better to fill it with track1.eta(), and where should I do that?

Comment on lines +285 to +288
registry.add("Nch_final_pt", "pT", {HistType::kTH1D, {axisEtaTrigger}});
registry.add("Solo_tracks_trigger", "pT", {HistType::kTH1D, {axisEtaTrigger}});
if (!cfgSingleSoloPtTrack) {
registry.add("Solo_tracks_assoc", "pT", {HistType::kTH1D, {axisEtaAssoc}});
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The solo-track QA histograms (Nch_final_pt, Solo_tracks_trigger, Solo_tracks_assoc) are created with eta axes (axisEtaTrigger/axisEtaAssoc) but are filled with track.pt() later. This makes these histograms incorrect; define them with a pT axis (e.g., axisPt) or change the fills to use eta if that was intended.

Suggested change
registry.add("Nch_final_pt", "pT", {HistType::kTH1D, {axisEtaTrigger}});
registry.add("Solo_tracks_trigger", "pT", {HistType::kTH1D, {axisEtaTrigger}});
if (!cfgSingleSoloPtTrack) {
registry.add("Solo_tracks_assoc", "pT", {HistType::kTH1D, {axisEtaAssoc}});
registry.add("Nch_final_pt", "pT", {HistType::kTH1D, {axisPt}});
registry.add("Solo_tracks_trigger", "pT", {HistType::kTH1D, {axisPt}});
if (!cfgSingleSoloPtTrack) {
registry.add("Solo_tracks_assoc", "pT", {HistType::kTH1D, {axisPt}});

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