-
Notifications
You must be signed in to change notification settings - Fork 620
[PWGJE] - updating stat prompt photon #9999
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
… incorporate data analysis
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
|
Error while checking build/O2Physics/o2 for 27cc197 at 2025-02-17 08:10: Full log here. |
|
Error while checking build/O2Physics/o2 for 80805f0 at 2025-02-17 08:17: Full log here. |
fjonasALICE
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,
I left some comments for your consideration. After they are implemented it should be okay for a first approval @nzardosh should check as well.
Best,
Florian
PWGJE/Tasks/statPromptPhoton.cxx
Outdated
| using MCClusters = o2::soa::Join<o2::aod::EMCALMCClusters, o2::aod::EMCALClusters>; | ||
| using selectedCollisions = soa::Join<aod::Collisions, aod::EvSels>; | ||
| using selectedMCCollisions = aod::McCollisions; | ||
| using Clusters = o2::aod::EMCALMCClusters; |
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.
@nzardosh is this one the correct one to use for derived JE data? Not sure but we so far anyways do not have derived data for MC? so should be fine?
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 am currently anyway not utilizing these non-JE derived classes, so it should be fine if I also remove it.
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.
For this we have a JE derived mode which is here:
https://github.com/AliceO2Group/O2Physics/blob/af190f20700d5e6323309f08789c99f4bd4c404b/PWGJE/DataModel/Jet.h#L208C1-L208C6
so this should be used
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.
These non-JE cluster classes are now removed
PWGJE/Tasks/statPromptPhoton.cxx
Outdated
| } // 3rd photon loop | ||
| } // 2nd photon check | ||
|
|
||
| std::cout << "We have a GEN prompt photon" << std::endl; |
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 probably do not want to havethese debug messages for every photon when running on hyperloop
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.
Done
| } | ||
| } // first photon loop | ||
|
|
||
| if (std::fabs(mompdg1) < 40 && std::fabs(mompdg1) > 0) { |
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.
can you explain the loop logic? it is not fully clear to me
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 is to check where the photon originates from, whether it is a "direct" prompt photon, originating from the hardest process, or is produced is subsequent showers. The statement simply ensures that the photon originates from something on the lower section of PDG codes (i.e., not form hadrons)
| if (!trackSelection(ogtrack)) { | ||
| continue; | ||
| } | ||
| if (!ogtrack.isGlobalTrack()) { |
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.
since you are in any case using derived data, I suggest to use the track selection from the jeutilities and make the type of track you want to filter on a configurable
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 would prefer not to do this. I prefer the flexibility to also be able to run over non-derived data.
Furthermore, I find this solution better when later on doing systematics on track quality, instead of using a magnitude of several restricted filters
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.
For this i would agree with Florian. If the only reason you need the original track table is track selection then you should instead use the je utilities and remove the subscription to the original tracks. Unless there is a type of track selection you need which we dont have (we can also add that to the JE framework). Using the JE utilities doesnt mean you are limited to the derived data, as you can still run over the original aod but it means you have the possibility of running over the derived too. If you keep the original tracks table you cant run over derived data
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.
For now, I have to insist on using the original track table.
I've already found that this analysis is extremely sensitive to the track selection used, (geometrical TPC cut is required, and the sensitivity of this cut is correlated to TPC crossed-rows etc) and I need more statistics to do proper QA in order to investigate which configuration of cuts work best.
It would at this point be inefficient use of my time to transfer to the je utilities, as I at this point don't know what kind of selection I would like to have, which would mean that I have to add 20-30 track selections to the je utilities that might be redundant in a couple of weeks.
Once the "golden cuts" + suitable systematic variations are found for the analysis, I will migrate to the je utilities and add any required track selection to the JE framework (as well as adding a flag for a built-in geometric TPC cut)
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.
If we can set a time, lets say a month where you can find the correct tracking cuts and then move to je utilities i think that is fine. But it would be quite important to do so as soon as possible, particularly before any analysis level long trains
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.
Yep, one month would be more than enough, I would essentially need like 10-20 or so subwagons running over the small apass7 dataset + small Jet-Jet anchored MC to estimate this. So one month would be fine for me.
PWGJE/Tasks/statPromptPhoton.cxx
Outdated
| histos.fill(HIST("REC_Track_v_Cluster_Phi_Eta_C"), phidiff, etadiff); | ||
| } else { | ||
| if (etatrigger && chargetrigger) { | ||
| std::cout << "????????????????????" << std::endl; |
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.
remove cout
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.
Done
| if (!trackSelection(ogtrack)) { | ||
| continue; | ||
| } | ||
| if (!ogtrack.isGlobalTrack()) { |
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 previous comment about track selection
PWGJE/Tasks/statPromptPhoton.cxx
Outdated
| { | ||
|
|
||
| nEventsData++; | ||
| if ((nEventsData + 1) % 10000 == 0) { |
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.
remove debug output, not needed when on hyperloop
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.
Done
| if (!trackSelection(ogtrack)) { | ||
| continue; | ||
| } | ||
| if (!ogtrack.isGlobalTrack()) { |
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 previous comment about track selection
PWGJE/Tasks/statPromptPhoton.cxx
Outdated
| double phidiff = TVector2::Phi_mpi_pi(cluster.phi() - ogtrack.phi()); | ||
| double etadiff = cluster.eta() - ogtrack.eta(); | ||
|
|
||
| if (fabs(etaT - etaC) < (0.010 + pow(ptT + 4.07, -2.5))) { |
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.
suggestion: make configurable
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.
Will do
| if (fabs(TVector2::Phi_mpi_pi(phiT - phiC)) < (0.015 + pow(ptT + 3.65, -2.0))) { | ||
| phitrigger = true; | ||
| } | ||
|
|
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.
E/p veto for track matching missing?
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.
No, E/p veto for track matching is later down the code (1213-1223)
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
[PWGJE] Please consider the following formatting changes to AliceO2Group#9999
|
Florian let me know when you are happy with the PR |
|
@nzardosh happy with the changes, you can approve! |
Dismissing since changes were adhered to
Co-authored-by: ALICE Action Bot <alibuild@cern.ch>
Update of stat prompt photon code to incorporate raw data-level analysis