-
Notifications
You must be signed in to change notification settings - Fork 613
[PWGJE] Replace TMath methods with O2 ones #12353
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
|
O2 linter results: ❌ 51 errors, |
vkucera
left a comment
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.
Hi @arvindkhuntia , thanks for the improvements.
Do you plan to fix the PDG codes in the rest of the code in this PR as well?
Please see my comments below.
PWGJE/Tasks/nucleiInJets.cxx
Outdated
| static constexpr int PDGDeuteron = o2::constants::physics::Pdg::kDeuteron; | ||
| static constexpr int PDGTriton = o2::constants::physics::Pdg::kTriton; | ||
| static constexpr int PDGHelium = o2::constants::physics::Pdg::kHelium3; |
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.
Use the existing PDG code names directly. Also for kProton.
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.
Hi @vkucera,
These constants are not used in the task. Removed them from the task. Thank you.
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.
But they should be used in mapPDGToValue.
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.
Indeed. I will fix this in my next PR.
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.
OK
| jetHist.fill(HIST("tracks/proton/h2TOFmass2ProtonVsPt_jet"), massTOF * massTOF - gMassProton * gMassProton, trk.pt()); | ||
| jetHist.fill(HIST("tracks/proton/h2TOFmass2ProtonVsPt_jet"), massTOF * massTOF - o2::constants::physics::MassProton * o2::constants::physics::MassProton, trk.pt()); |
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.
You can put
using namespace o2::constants::physics;in your task to avoid spelling the prefix every time.
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.
Thanks for the suggestion. I will fix this in the next PR.
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.
OK
Co-authored-by: Arvind Khuntia <arvind.khuntia@cern.ch>
Co-authored-by: Arvind Khuntia <arvind.khuntia@cern.ch>
Co-authored-by: Arvind Khuntia <arvind.khuntia@cern.ch>
Dear @nzardosh,
I have removed TMath dependency and tried to solve some of the issues related to the O2 linter in this PR (Thanks to @vkucera for suggestions). Please approve the PR once tests are passed.
Have a nice afternoon.