-
Notifications
You must be signed in to change notification settings - Fork 485
gammaTask first prototype #6034
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
4cc4182 to
867100c
Compare
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.
This has to be debug I guess
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.
please use LOGF
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.
Please change ATask to a meaningful name, and then you can drop the second argument (TaskName)
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 meaningful help strings (third argument) for all configurables
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 LOGF everywhere
jgrosseo
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.
Thanks for this first version. I have posted some comments, in addition to the ones of @iarsene
In addition: it is not needed to have an h and cxx file for the task. Put everything in the cxx file (as for the other analysis tasks).
|
This PR did not have any update in the last 30 days. Is it still needed? Unless further action in will be closed in 5 days. |
|
@sstiefel19 Is there any update ? i received the 5 days warning before this PR will be automatically closed. |
867100c to
ce50fdc
Compare
| aod::McParticles const& theMcParticles) | ||
| { | ||
| for (auto& lV0 : theV0s) { | ||
| float lPhiV0Rec = static_cast<float>(M_PI) + std::atan2(-lV0.py(), -lV0.px()); |
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.
just saw this quantity is also included in the V0s now. so I will take it from there with the next version
a9a2c3a to
dd8a934
Compare
dd8a934 to
424d6ce
Compare
add a cut on found over findable tpc clusters update two tablenames
| { | ||
| {"IsPhotonSelected", "IsPhotonSelected", {HistType::kTH1F, {{12, -0.5f, 11.5f}}}}, | ||
|
|
||
| {"beforeCuts/hPtRec_before", "hPtRec_before", {HistType::kTH1F, {{100, 0.0f, 25.0f}}}}, |
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.
Why not skip _before here, you have them already in the folder "beforeCuts"?
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.
I wanted to keep it in case I quickly plot its still directly visible what it is. But if wanted I can of course remove the _before, let me know
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.
I meant to do: beforeCuts/hPtRec", "hPtRec_before
In this case it should be in the title, but not in the object name (but that one is always in the folder, so it is clear)
jgrosseo
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.
See my two minor comments for consideration. I start the CI.
add cut on TPCCrossedRowsOverFindableCls add TPCFoundOverFindableCls to TracksExtra
remove _before in histo names
remove capital letters in executable name
before templating data/mc