-
Notifications
You must be signed in to change notification settings - Fork 0
[PWGCF] Add files via upload #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request modifies the etaDihadron.cxx file to change histogram tracking from transverse momentum (pT) to pseudorapidity (eta) for trigger and associated particles in di-hadron correlation analysis. The changes affect histogram definitions, axis specifications, and data filling calls.
Changes:
- Updated histogram axes and fill calls from pT to eta for trigger histograms (Trig_hist, MCTrig_hist)
- Changed histogram names and titles from "pt" to "eta" for solo track histograms (Nch_final_eta, Solo_tracks_trigger, Solo_tracks_assoc)
- Minor formatting fix removing space in cast expression
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (std::find(tracksSkipIndices.begin(), tracksSkipIndices.end(), track1.globalIndex()) != tracksSkipIndices.end()) { | ||
| registry.fill(HIST("Solo_tracks_trigger"), track1.pt()); | ||
| registry.fill(HIST("Solo_tracks_trigger"), track1.eta()); | ||
| continue; // Skip the track1 if it is alone in pt bin |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "Skip the track1 if it is alone in pt bin" but the code has been changed to fill histograms with eta values instead of pt values. However, the underlying logic in findBiasedTracks (line 838-875) still identifies tracks that are alone in their pT bins, not eta bins. This creates a semantic inconsistency - the function identifies solo tracks based on pT binning, but the histogram is now filled with eta values. Either the comment should be updated to clarify that tracks are identified as solo based on pT bins but their eta is recorded, or the entire feature should be reconsidered for consistency.
| if (std::find(tracks2SkipIndices.begin(), tracks2SkipIndices.end(), track2.globalIndex()) != tracks2SkipIndices.end()) { | ||
| registry.fill(HIST("Solo_tracks_assoc"), track2.pt()); | ||
| registry.fill(HIST("Solo_tracks_assoc"), track2.eta()); | ||
| continue; // Skip the track2 if it is alone in pt bin |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "Skip the track2 if it is alone in pt bin" but the code has been changed to fill histograms with eta values instead of pt values. However, the underlying logic in findBiasedTracks (line 838-875) still identifies tracks that are alone in their pT bins, not eta bins. This creates a semantic inconsistency - the function identifies solo tracks based on pT binning, but the histogram is now filled with eta values. Either the comment should be updated to clarify that tracks are identified as solo based on pT bins but their eta is recorded, or the entire feature should be reconsidered for consistency.
| registry.add("Nch_final_eta", "eta", {HistType::kTH1D, {axisEtaTrigger}}); | ||
| registry.add("Solo_tracks_trigger", "eta", {HistType::kTH1D, {axisEtaTrigger}}); | ||
| if (!cfgSingleSoloPtTrack) { | ||
| registry.add("Solo_tracks_assoc", "pT", {HistType::kTH1D, {axisEtaAssoc}}); | ||
| registry.add("Solo_tracks_assoc", "eta", {HistType::kTH1D, {axisEtaAssoc}}); |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The histogram names have been changed from "Nch_final_pt", "Solo_tracks_trigger", and "Solo_tracks_assoc" to use "eta" instead of "pt", and they are now filled with eta values. However, the configuration variable cfgSoloPtTrack (line 103) and the function findBiasedTracks (line 838-875) still refer to pT bins. This creates semantic confusion - the feature is configured and documented as being about pT bins, but the histograms now record eta values. Consider either: (1) keeping the original pT-based histogram fills to maintain consistency with the feature's purpose, or (2) if eta tracking is intentional, update the configuration variable names and function names to reflect this change in behavior.
| @@ -282,10 +282,10 @@ struct EtaDihadron { | |||
| registry.add("Trig_hist", "", {HistType::kTHnSparseF, {{axisSample, axisVertex, axisEtaTrigger}}}); | |||
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The histogram definition now uses axisEtaTrigger and tracks eta values, which is inconsistent with a similar implementation in PWGCF/TwoParticleCorrelations/Tasks/diHadronCor.cxx (lines 282, 285-288) where Trig_hist and solo track histograms use axisPtTrigger and track pT values. If this divergence is intentional for eta-specific analysis, consider documenting the rationale. Otherwise, these implementations should remain consistent across the codebase.
No description provided.