Skip to content

Conversation

@stefanopolitano
Copy link
Collaborator

This PR adds the possibility to process Phi and V0s in the filter2Prong code. PID selections can be applied at the filterCorrelation and filter2Prong levels. @skundu692 @jgrosseo @prottayCMT for your information.

@github-actions
Copy link

github-actions bot commented Jun 26, 2025

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

Copy link
Contributor

@jgrosseo jgrosseo left a comment

Choose a reason for hiding this comment

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

Thanks a lot. Here some comments!

Copy link
Contributor

@jgrosseo jgrosseo left a comment

Choose a reason for hiding this comment

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

Great work! A few more comments inline, then we can merge... :)


if (grpPhi.DoPhi) { // Process Phi mesons
for (const auto& cftrack1 : cftracks) { // Loop over first track
const auto& p1 = tracks.iteratorAt(cftrack1.trackId() - tracks.begin().globalIndex());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we use?

Suggested change
const auto& p1 = tracks.iteratorAt(cftrack1.trackId() - tracks.begin().globalIndex());
const auto& p1 = cftrack1.track();

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure this can be done straightforwardly. As far as I understood, cftrack1 only contains the global track and collision indexes, while the physics information are stored in the CFTracks (which however do not contain global index information, but only cf-index ones) and PIDTrack tables. We might need to modify the tables in order to have both the physics and global index information to do it more efficiently than how it is currently done. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree with your statement (Track in CFTrack is an index to the global track to my understanding) but I don't want to delay this PR. We can try it together one of these days.

Comment on lines 177 to 180
/*
template <typename T>
using HasProtonPID = decltype(selectionPIDProton(std::declval<T&>()));
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we remove the commented lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

}

void processData(soa::Filtered<soa::Join<aod::Collisions, aod::EvSels, aod::CFMultiplicities>>::iterator const& collision, aod::BCsWithTimestamps const&, soa::Filtered<soa::Join<aod::Tracks, aod::TracksExtra, aod::TrackSelection>> const& tracks)
void processData(soa::Filtered<soa::Join<aod::Collisions, aod::EvSels, aod::CFMultiplicities>>::iterator const& collision, aod::BCsWithTimestamps const&, soa::Filtered<soa::Join<aod::Tracks, aod::TracksExtra, aod::TrackSelection, aod::pidTPCPr, aod::pidTOFPr, aod::TracksDCA>> const& tracks)
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly now one cannot use this task anymore without PID information? I think that is not good.
We have to make 2 process functions which then can call a templated one, to avoid the code duplication...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I split the process function, one for data (default) and one for data pid. They both access a common templated function. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you tested both?

Copy link
Contributor

@jgrosseo jgrosseo left a comment

Choose a reason for hiding this comment

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

Great! I will now seek feedback from @jaelpark as well.
You can already fix the clang in the meanwhile?
We can then remove the draft status.


if (grpPhi.DoPhi) { // Process Phi mesons
for (const auto& cftrack1 : cftracks) { // Loop over first track
const auto& p1 = tracks.iteratorAt(cftrack1.trackId() - tracks.begin().globalIndex());
Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree with your statement (Track in CFTrack is an index to the global track to my understanding) but I don't want to delay this PR. We can try it together one of these days.

}

void processData(soa::Filtered<soa::Join<aod::Collisions, aod::EvSels, aod::CFMultiplicities>>::iterator const& collision, aod::BCsWithTimestamps const&, soa::Filtered<soa::Join<aod::Tracks, aod::TracksExtra, aod::TrackSelection>> const& tracks)
void processData(soa::Filtered<soa::Join<aod::Collisions, aod::EvSels, aod::CFMultiplicities>>::iterator const& collision, aod::BCsWithTimestamps const&, soa::Filtered<soa::Join<aod::Tracks, aod::TracksExtra, aod::TrackSelection, aod::pidTPCPr, aod::pidTOFPr, aod::TracksDCA>> const& tracks)
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you tested both?

O2_DEFINE_CONFIGURABLE(cfgCutEta, float, 0.8f, "Eta range for tracks")

Filter trackFilter = (nabs(aod::track::eta) < cfgCutEta) && (aod::track::pt > cfgCutPt);
Filter trackSelection = (requireGlobalTrackInFilter()) || (aod::track::isGlobalTrackSDD == (uint8_t) true);
Copy link
Collaborator

@jaelpark jaelpark Jul 7, 2025

Choose a reason for hiding this comment

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

There's a problem with the clang-format and the subsequent checks, they are not compatible. To pass the test, this needs to be manually changed back to (uint8_t) true (with the space) each time before commit (same for the other one higher up).

@stefanopolitano
Copy link
Collaborator Author

Hi @jaelpark ! Thanks a lot for your suggestions, I implemented them in the last commit. Let me know if you have further comments

@jgrosseo jgrosseo marked this pull request as ready for review July 7, 2025 14:57
Please consider the following formatting changes to AliceO2Group#11774
@jaelpark
Copy link
Collaborator

jaelpark commented Jul 7, 2025

Thanks! No other comments from me.

@jgrosseo jgrosseo enabled auto-merge (squash) July 8, 2025 08:22
@jgrosseo jgrosseo merged commit bb027af into AliceO2Group:master Jul 8, 2025
11 of 12 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.

5 participants