-
Notifications
You must be signed in to change notification settings - Fork 497
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
introducing gamma-jet tree producer task #7367
Conversation
fjonasALICE
commented
Aug 20, 2024
- addition of new workflow which allows to store a tree containing photons and jets, including isolation information for the jet. Ideal derived data to perform any gamma-jet analysis
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
// an integer instead | ||
Filter clusterDefinitionSelection = (o2::aod::jcluster::definition == mClusterDefinition); | ||
// Process clusters | ||
void processClusters(soa::Join<JetCollisions, aod::BkgChargedRhos, aod::JCollisionBCs>::iterator const& collision, aod::JBCs const&, selectedClusters const& clusters, JetTracks const& tracks) |
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 remove JBCs once you remove zorro
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 Florian this is not resolved yet
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.
removed JBCs
Please consider the following formatting changes to AliceO2Group#7367
Hi @nzardosh I implemented almost all your comments, except for the filter for jet pt, which i will implement later (but i think it is not crucial) |
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
// an integer instead | ||
Filter clusterDefinitionSelection = (o2::aod::jcluster::definition == mClusterDefinition); | ||
// Process clusters | ||
void processClusters(soa::Join<JetCollisions, aod::BkgChargedRhos, aod::JCollisionBCs>::iterator const& collision, aod::JBCs const&, selectedClusters const& clusters, JetTracks const& tracks) |
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 Florian this is not resolved yet
PWGJE/Tasks/CMakeLists.txt
Outdated
o2physics_add_dpl_workflow(gamma-jet-tree-producer | ||
SOURCES gammajettreeproducer.cxx | ||
PUBLIC_LINK_LIBRARIES O2::Framework O2::EMCALBase O2::EMCALCalib O2Physics::PWGJECore O2Physics::AnalysisCore | ||
O2Physics::EventFilteringUtils |
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 think this is not needed anymore since you dont interface with zorro
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.
both removed
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
using namespace o2; | ||
using namespace o2::framework; | ||
using namespace o2::framework::expressions; | ||
using bcEvSelIt = o2::soa::Join<o2::aod::BCs, o2::aod::BcSels>::iterator; |
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 isnt used anywhere
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.
removed
Configurable<float> isoR{"isoR", 0.4, "isolation cone radius"}; | ||
|
||
// cluster cuts | ||
Configurable<int> mClusterDefinition{"clusterDefinition", 10, "cluster definition to be selected, e.g. 10=kV3Default"}; |
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.
might be more useful to take this input as a string so that if the definitions change you'e still safe. you can see how in the jetfinder
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.
prefer to keep consistent with other tasks for now, will change eventually
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
|
||
double dPhi = TVector2::Phi_0_2pi(phi1) - TVector2::Phi_0_2pi(phi2); | ||
if (abs(dPhi) > M_PI) { | ||
dPhi = 2 * M_PI - abs(dPhi); |
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 deltaR function from jetutilities. you can give it to objects and it gives you back the DeltaR
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.
replaced with deltaR function. I did some additional fixes to deltaR function in jetutilities that should improve convenience and safety
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
if (collision.posZ() > mVertexCut) { | ||
return false; | ||
} | ||
if (!jetderiveddatautilities::selectCollision(collision, eventSelection) || !jetderiveddatautilities::selectTrigger(collision, triggerMaskBits)) { |
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.
in the jet finder i dont think we apply collision selection in parallel to emcal selections. Check that this is what you want
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 leave for now but will clarify with Nicolas Strangmann for a future update
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
// dEta = 0; | ||
// } | ||
|
||
gammasTable(collisionMapping[collision.globalIndex()], cluster.energy(), cluster.eta(), cluster.phi(), cluster.m02(), cluster.m20(), cluster.nCells(), cluster.time(), cluster.isExotic(), cluster.distanceToBadChannel(), cluster.nlm(), isoraw, perpconerho, dPhi, dEta, p); |
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.
here I am not sure you can get the table index via collisionMapping[collision.globalIndex()]
You should use the find function and then get the second element of the pair returned with a protection that it doesnt reach the end.
For here however you can actually just use eventsTable.lastIndex() since it 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.
fixed and changed to unordered map to improve performance
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
|
||
Filter jetCuts = aod::jet::pt > jetPtMin&& aod::jet::r == nround(jetR.node() * 100.0f); | ||
// Process charged jets | ||
void processChargedJets(soa::Join<JetCollisions, aod::BkgChargedRhos, aod::JCollisionBCs>::iterator const& collision, aod::JBCs const&, soa::Filtered<soa::Join<aod::ChargedJets, aod::ChargedJetConstituents>> const& chargedJets, JetTracks const&) |
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 JBCs
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.
removed
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
nconst++; | ||
} | ||
|
||
chargedJetsTable(collisionMapping[collision.globalIndex()], jet.pt(), jet.eta(), jet.phi(), jet.energy(), jet.mass(), jet.area(), nconst); |
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.
here get the pair according to the comment above. If you need an example you can look in the derived data write. I'm surprised it compiles actually.
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.
fixed
PWGJE/Tasks/gammajettreeproducer.cxx
Outdated
WorkflowSpec defineDataProcessing(ConfigContext const& cfgc) | ||
{ | ||
WorkflowSpec workflow{ | ||
adaptAnalysisTask<GammaJetTreeProducer>(cfgc, TaskName{"gamma-jet-tree-producer"}, SetDefaultProcesses{{{"processClearMaps", true}, {"processClusters", true}, {"processChargedJets", 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.
instead of adding the process functions as true here you can instead remove it here and add them as true in the process switches
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.
fixed
Please consider the following formatting changes to AliceO2Group#7367
* introducing gamma-jet tree producer task * fix missing new line in gj tree producer * update of software triggers for GJ tree * delection of obsolete table subscription, map improvements * Please consider the following formatting changes --------- Co-authored-by: ALICE Action Bot <[email protected]>