Uh oh!
There was an error while loading. Please reload this page.
[PWGJE] Add Jet-hadron correlations task into the JE framework - #13847
Conversation
O2 linter results: ❌ 1 errors, |
Please consider the following formatting changes to AliceO2Group#13847
nzardosh
commented
Nov 17, 2025
Hi. Please squash your commits so we can perform a review |
yhou-git
commented
Nov 18, 2025
Hello Nima, I'm not quite ready yet, but I will be ready for review once it's sorted. Thanks, Yongzhen |
7ea1e7c to
80fcbc9Comparevkucera
commented
Nov 18, 2025
@yhou-git Please use a meaningful PR title. |
yhou-git
commented
Nov 18, 2025
Hi Vit, thanks for your reminder, have updated it, thanks a lots. Yongzhen |
vkucera
commented
Nov 18, 2025
Thanks to you! |
nzardosh
commented
Nov 20, 2025
Hi @yhou-git thanks for letting me know. In general if you are developing your own task its best not to open the PR until you have it ready. In case you are working on a core part of the framework and need intermediate reviews you can open the PR in draft mode |
yhou-git
commented
Nov 20, 2025
Hi @nzardosh, thanks! My task is now ready for review. There are no further changes planned at the moment. please let me know if anything needs adjustment. Thanks again, yongzhen |
Uh oh!
There was an error while loading. Please reload this page.
9812168 to
8331d11Compare
nzardosh
left a comment
There was a problem hiding this comment.
Dear @yhou-git Thanks for the PR. In general I think the code is quite a bit more complicated than it needs to be in places. I have tried to highlight examples so you can go through the code and fix each time the example appears. Thanks!
| template <typename TCollision> | ||
| float getCentrality(const TCollision& coll) const | ||
| { | ||
| if (cfgCentEstimator.value == 0) |
There was a problem hiding this comment.
why do you need the .value here instead of calling the variable directly?
There was a problem hiding this comment.
I need this centrality value to work for different systems. Pb-Pb uses FT0C for centrality, but pp uses FT0M.
Using .value makes the code flexible for both, or other systems.
| // ========================================================== | ||
| template <typename TCollision> | ||
| bool isGoodCollision(const TCollision& coll, | ||
| const std::vector<int>& eventSelectionBits, bool skipMBGapEvents, |
There was a problem hiding this comment.
all the variables other than coll are defined globally in your struct and do not need to be passed to the function
There was a problem hiding this comment.
I see your point and will test it. My idea was to simplify the selections inside each process.
| return false; | ||
| } | ||
| float cent = getCentrality(coll); | ||
| if (cent < centralityMin || cent > centralityMax) { |
There was a problem hiding this comment.
isnt this already taken care of by the eventCuts filter?
There was a problem hiding this comment.
Yes, it is already included.
I only added the extra lines as an additional safety check.
| if (cent < centralityMin || cent > centralityMax) { | ||
| return false; | ||
| } | ||
| if (std::abs(coll.posZ()) > vertexZCut) { |
There was a problem hiding this comment.
this can go in the eventCuts filter too
There was a problem hiding this comment.
You are right, let me consider whether to retain them or remove them. I shall first test whether removing them changes my results.
| double constituentPtMin = -98.0; | ||
| double constituentPtMax = 9998.0; | ||
| if (jetAreaFractionMin > jetAreaLimit) { | ||
| if (jet.area() < jetAreaFractionMin * o2::constants::math::PI * (jet.r() / 100.0) * (jet.r() / 100.0)) { |
There was a problem hiding this comment.
this cut is typically done in the jetFinder already, but you can do it here too if you want to be safe
There was a problem hiding this comment.
Thanks very much for pointing it out. I will check if it influences.
| auto tracksTuple = std::make_tuple(jets, tracks); | ||
| Pair<TCollisions, TJets, TTracks, BinningType> pairData{corrBinning, numberEventsMixed, -1, collisions, tracksTuple, &cache}; | ||
| for (const auto& [c1, jets1, c2, tracks2] : pairData) { |
There was a problem hiding this comment.
how does this work? what is it iterating over?
There was a problem hiding this comment.
This part follows the event mixing tutorial.
It builds the mixing pools and iterates over the mixed event pairs.
https://github.com/AliceO2Group/O2Physics/blob/master/Tutorials/src/eventMixing.cxx#L220C25-L220C28.
| if (!jetderiveddatautilities::selectCollision(collision, eventSelectionBits, skipMBGapEvents)) { | ||
| continue; | ||
| } | ||
| if (collision.trackOccupancyInTimeRange() < trackOccupancyInTimeRangeMin || trackOccupancyInTimeRangeMax < collision.trackOccupancyInTimeRange()) { |
There was a problem hiding this comment.
Ok, I see and will try to improve it, thanks
| if (acceptSplitCollisions == SplitOkCheckFirstAssocCollOnly || acceptSplitCollisions == NonSplitOnly) { | ||
| centrality = getCentrality(collisions.begin()); | ||
| multiplicity = getMultiplicity(collisions.begin()); |
There was a problem hiding this comment.
does the SmallGroups essentially slice the collision table?
There was a problem hiding this comment.
I think so, smallgroups gives only the collisions associated to the current mccollision.
| } | ||
| } | ||
| } | ||
| if (!hasSel8Coll || !centralityIsGood || !occupancyIsGood) { |
There was a problem hiding this comment.
instead of doing this you can put a return in each if statement. For example
if (!jetderiveddatautilities::selectCollision(collision, eventSelectionBits, skipMBGapEvents)) {
return;
}
| } | ||
| float jetweight = jet.eventWeight(); | ||
| double pTHat = 10. / (std::pow(jetweight, 1.0 / pTHatExponent)); | ||
| for (int N = 1; N < ptHadbins; N++) { |
There was a problem hiding this comment.
what is this pTHadbins doing?
There was a problem hiding this comment.
Sorry, the purpose was to perform the JJ samples. But I have not updated this section recently. I would like to first review my data points, mainly focusing on the Data and MCD levels. I will update this process accordingly. This pTHadbins value and the related loop will also be removed.
yhou-git
commented
Nov 26, 2025
Dear Nima @nzardosh, thank you very for the review and suggestions. I will go through this task, simplify it as recommended, and update the PR. I will let you know once the optimizations are completed. Thanks again. |
0e8106f to
d7a9debCompareUh oh!
There was an error while loading. Please reload this page.
No description provided.