Skip to content

Conversation

@jkinner2
Copy link
Contributor

@jkinner2 jkinner2 commented Jun 2, 2025

Adding task for analysing angular photon-jet correlations in data (PWG-JE JP PAG presentation: https://indico.cern.ch/event/1546154/#22-photon-hadron-correlations)

@github-actions github-actions bot added the pwgje label Jun 2, 2025
@github-actions
Copy link

github-actions bot commented Jun 2, 2025

O2 linter results: ❌ 1 errors, ⚠️ 0 warnings, 🔕 0 disabled

@github-actions github-actions bot changed the title Add photon-hadron correlation analysis task [PWGJE] Add photon-hadron correlation analysis task Jun 2, 2025
@jkinner2 jkinner2 marked this pull request as ready for review June 2, 2025 13:49
@jkinner2
Copy link
Contributor Author

jkinner2 commented Jun 2, 2025

The failed O2 linter check seems to stem only from other workflows in PWGJE/Tasks/CMakeLists.txt

nzardosh
nzardosh previously approved these changes Jun 2, 2025
Copy link
Collaborator

@nzardosh nzardosh left a comment

Choose a reason for hiding this comment

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

Thanks for the work and welcome to analysis on O2 :) I have left a very short guiding review, however you can wait for a more detailed review from the EMCal side by @fjonasALICE . For now the main issues I spot is the way the JE derived tables are interfaced with the non-JE tables and also that it seems in more places JE tables could be used. Thanks

PROCESS_SWITCH(CorrelationTableProducer, processRecoCollisionTrigger, "process correlation collision_extra and trigger table (reconstructed)", false);

void processRecoPipmTPCTOF(aod::JetCollision const& collision,
soa::Join<aod::JetTracks, aod::TracksExtra, aod::TrackSelection, aod::pidTPCPi, aod::pidTOFPi> const& tracks)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The JetTracks table is not joinable by default to the other Track tables. You need to link them through the JTracksPI table as shown in the jettutorial task

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have adjusted the task in the commit "PR_adjustments"

// hadron/pipm
for (auto const& track : tracks) {
// track selection
if (!checkGlobalTrackEta(track))
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use the track selections implemented in the JE framework

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adjusted in the commit "PR_adjustments"

return;

// photonsPCM (for some reason collsionId not an index column (?))
auto const v0PhotonsThisEvent = v0Photons.sliceBy(perColV0Photons, collision.collision().globalIndex());
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of collision.collision().globalIndex() you can do collision.collisionId()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adjusted in the commit "PR_adjustments"

auto const v0PhotonsThisEvent = v0Photons.sliceBy(perColV0Photons, collision.collision().globalIndex());

// photonPCM
for (auto const& v0Photon : v0PhotonsThisEvent) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

where are the v0Photons produced?I guess these are objects from LF. @fjonasALICE do we expect to use these or do we have our own photon/JCluster tables that can be used here instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

these are not emcal and i am no expert for the V0 in O2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reply in the original comment below

}
PROCESS_SWITCH(PhotonChargedTriggerCorrelation, processCorrFirst, "process to gather info before correlation processes", false);

void processCorrHadron(CorrCollision const& collision, aod::Triggers const& triggers, aod::Hadrons const& hadrons)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this trying to use triggered data? If so please use the JE methods and tables for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained in the original comment, the task is not built for running on triggered data, so I would not know what tables to include. The only software-trigger-related part of the code is an addition in the event selection (used for a test and disabled by default) via "triggerMaskBits" (as in the jetTutorial task).

@nzardosh nzardosh self-requested a review June 2, 2025 14:42
@jkinner2
Copy link
Contributor Author

jkinner2 commented Jun 3, 2025

Hello, thank you for the quick response. I have adjusted the task.
Regarding your questions:
The V0PhotonsKF table is produced by the EM task photon-conversion-builder for PCM photons. My task currently does not use the EMCal.
The Triggers table only refers to tracks above some pt threshold. The task runs on minimum bias data (possibly on triggered data in the future).

@jkinner2
Copy link
Contributor Author

@nzardosh @fjonasALICE
It has been about two weeks since the last update on this PR. I am following up in case this was missed or if there is any further feedback or anything else needed from my side

Copy link
Collaborator

@fjonasALICE fjonasALICE left a comment

Choose a reason for hiding this comment

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

Hi
@nzardosh since no emcal is used I can't comment a lot on the V0. I went through the code for a few general comments. For detailled review the number of changes is too large, but if the code is tested, and given it does not touch anything else, I think it is fine to approve once the requested changes are in. I also did not see if @jkinner2 implemented the changes for the JE software triggers, and one should double check that the V0 candidates can be properly matched to the index of JCollisions

auto const v0PhotonsThisEvent = v0Photons.sliceBy(perColV0Photons, collision.collision().globalIndex());

// photonPCM
for (auto const& v0Photon : v0PhotonsThisEvent) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

these are not emcal and i am no expert for the V0 in O2

/// Also contains checks and monte-carlo (efficiency, purity, mc-true correlation,...)
/// End goal of studying correlations between direct photons and jets

#define ETA_MAX_DEFAULT 0.8
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we really need to define like this? I think a const double etc in the current namespace would be the better idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented it in the commit "change_define_to_variable__move_table_definitions_to_own_header"

// derived data for correlations (on-the-fly)
// should be more efficient due to usage in correlations and event mixing

namespace o2::aod
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would propose to move table definitions to their own header file, like it is done for other analysis task

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have adjusted the task in the commit "change_define_to_variable__move_table_definitions_to_own_header"

@jkinner2
Copy link
Contributor Author

jkinner2 commented Jul 1, 2025

@nzardosh @fjonasALICE
Apologies for previously answering in a central comment. I have now replied to the individual conversation threads.
Regarding the V0 candidate matching: The collision matching is done with the JCollisionPIs table, similar to the track-linking for the PID task. From my tests of the task, I am confident that the V0s are matched correctly.
Please let me know if there are any further changes that need to be made

@nzardosh nzardosh enabled auto-merge (squash) July 7, 2025 12:28
@nzardosh nzardosh merged commit 9476b96 into AliceO2Group:master Jul 7, 2025
11 of 13 checks passed
jpxrk pushed a commit to jpxrk/O2Physics that referenced this pull request Jul 16, 2025
prottayCMT pushed a commit to prottayCMT/O2Physics2024 that referenced this pull request Jul 18, 2025
JimunLee pushed a commit to JimunLee/O2Physics that referenced this pull request Jul 22, 2025
vojmach pushed a commit to vojmach/O2Physics that referenced this pull request Jul 23, 2025
jloemker pushed a commit to jloemker/O2Physics that referenced this pull request Nov 11, 2025
alibuild pushed a commit to alibuild/O2Physics that referenced this pull request Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants