[PWGJE] Task for the nuclei study with FastJet framework #7213
Conversation
arvindkhuntia
requested review from
alibuild,
maoyx,
nzardosh,
ddobrigk and
mfasDa
as code owners
Clang-format
Please consider the following formatting changes to #7213
|
Error while checking build/O2Physics/o2 for a81fd66 at 2024-08-08 00:19: Full log here. |
|
Error while checking build/O2Physics/o2 for ed2cd19 at 2024-08-08 00:38: Full log here. |
There was a problem hiding this comment.
Thanks alot for making the PR and welcome to jet analyses! The code, particularly many of the histograms repeated for candidates can be simplified by using one higher dimensional histogram. I left comments in particular process functions but were appropriate those comments should be applied to all process functions.
Also at a later time I might consider adding some of the additional params you take from the full track table to the jet derived data format. But for now I think using the full track table is fine. I guess the pT cut you will use in the analysis is not so high?
| // add pid later | ||
|
|
||
| bool jetFlag = false; | ||
| for (int iDJet = 0; iDJet < mcpJetPt.size(); iDJet++) { |
There was a problem hiding this comment.
why dont you just loop over the mcp jets here and her eta and phi directly from the object?
There was a problem hiding this comment.
same with the process function above
There was a problem hiding this comment.
Thank you very much @nzardosh for the comments. Here, I loop over only the matched jets instead of looping over the all mcp jets.
| continue; | ||
| if (fabs(fullTrack.eta()) > cfgtrkMaxEta) | ||
| continue; | ||
| if (!track.has_mcParticle()) |
There was a problem hiding this comment.
you can also get these parameters from JParticles. Then you wouldnt need to subscribe to the aod::McParticles table
There was a problem hiding this comment.
It is now done.
PWGJE/Tasks/nucleiWithFastJet.cxx
Outdated
|
|
||
| if (R < cfgjetR) { | ||
| jetFlag = true; | ||
| // break; |
There was a problem hiding this comment.
why do you not break here?
PWGJE/Tasks/nucleiWithFastJet.cxx
Outdated
| } // jet | ||
|
|
||
| if (track.mcParticle().pdgCode() == PDGProton) { | ||
| jetHist.fill(HIST("mcdJet/proton/pt/PtProton"), track.mcParticle().pt()); |
There was a problem hiding this comment.
you can probably replace all these histograms below and all the if conditions with just one 3D histogram with axes of pt, jetFlag and pdgCode where you use an enum to map the pdg codes yourself. Then you have 1 histogram and just one line like this
jetHist.fill(HIST("mcdJet/pt/PtPatritcleType"), track.mcParticle().pt(), jetFlag, mapPDGCodes(track.mcParticle().pdgCode()));
This also applies to the other process functions above as well
There was a problem hiding this comment.
Thanks for the suggestion. I have now implemented a mapping for the hist in the MC process functions.
Please consider the following formatting changes to #7213
PWGJE/Tasks/nucleiWithFastJet.cxx
Outdated
| } else { | ||
| jetHist.fill(HIST("mcdJet/proton/pt/PtProton_outJet"), track.mcParticle().pt()); | ||
| } | ||
| if (mapPDGToValue(mcTrack.pdgCode())) { |
There was a problem hiding this comment.
what does this line do? because it can be negative i am not sure how it works. Maybe instead you can have if (mapPDGToValue(mcTrack.pdgCode()) != 0)
There was a problem hiding this comment.
Thanks @nzardosh for the suggestion, with the current version the if statement is false only for 0 and true for any other value. For clarity, I have impemented your suggestion.
| jetHist.add<TH2>("tracks/h2TOFbetaVsP", "TOF #beta vs #it{p}/Z; Signed #it{p} (GeV/#it{c}); TOF #beta", HistType::kTH2F, {{250, -5.f, 5.f}, {betaAxis}}); | ||
|
|
||
| // TOF hist | ||
| jetHist.add<TH2>("tracks/proton/h2TOFmassProtonVsPt_jet", "h2TOFmassProtonVsPt_jet; TOFmass; #it{p}_{T} (GeV)", HistType::kTH2F, {{80, 0.4, 4.}, {50, 0., 5.}}); |
There was a problem hiding this comment.
All the 2D histograms with particle species could also be changed to 3D ones with the new pdgcode switches you have
There was a problem hiding this comment.
this would make the code much simpler i think
There was a problem hiding this comment.
Thanks for the suggestion. I plan to add the pt of the jet to the 2D histos in my next iteration. Therefore, I will keep this as it is and try to optimise it later.
Please consider the following formatting changes to #7213
|
Dear @nzardosh, could you please approve the PR if all the implementations are fine with you. Thanks. |
…p#7213) * [PWGJE] Task for the nuclei study with FastJet framework * Please consider the following formatting changes * Megalinter fixes * Megalinter fixes * Update nucleiWithFastJet.cxx * Please consider the following formatting changes * Update nucleiWithFastJet.cxx * Update nucleiWithFastJet.cxx * Comments addressed * Please consider the following formatting changes * Comment addressed * Please consider the following formatting changes * Task and executable name changed --------- Co-authored-by: Arvind Khuntia <[email protected]> Co-authored-by: ALICE Action Bot <[email protected]>
…p#7213) * [PWGJE] Task for the nuclei study with FastJet framework * Please consider the following formatting changes * Megalinter fixes * Megalinter fixes * Update nucleiWithFastJet.cxx * Please consider the following formatting changes * Update nucleiWithFastJet.cxx * Update nucleiWithFastJet.cxx * Comments addressed * Please consider the following formatting changes * Comment addressed * Please consider the following formatting changes * Task and executable name changed --------- Co-authored-by: Arvind Khuntia <[email protected]> Co-authored-by: ALICE Action Bot <[email protected]>
Dear @nzardosh,
Please have a look at the PR.
Many thanks in advance.