Fixes and extensions for gen particle collection #49
Conversation
…rst layer of the HGCal. Added switch for both
There was a problem hiding this comment.
Code looks good to me, haven't tested it myself though. Could you please also update https://github.com/CMS-HGCAL/reco-ntuples/blob/master/Definitions.md ?
@artlbv and @beaudett might want to add.
| @@ -94,8 +188,10 @@ void computeWidth(const reco::HGCalMultiCluster& cluster, math::XYZPoint & bar, | |||
|
|
|||
|
|
|||
| // ---------parameters ---------------------------- | |||
| bool readOfficialReco; | |||
| //bool readOfficialReco; | |||
There was a problem hiding this comment.
Please delete lines referring to readOfficialReco instead of commenting them out.
There was a problem hiding this comment.
Why getting rid of this flag and the associated possibility to read either simTracks/Vertices or TrackingParticles/Vertices? Can't we keep both?
There was a problem hiding this comment.
The tracking particles collection did not include all generated particles. That was the reason to switch.
There was a problem hiding this comment.
@rovere Do you see any advantage in keeping both collections or a use case for TrackingParticles/Vertices?
There was a problem hiding this comment.
maybe @malgeri can comment better on this topic.
|
|
||
| if(std::abs(myTrack.vertex().position().z())>=layerPositions[0]) continue; | ||
|
|
||
| /* |
| mySimEvent->fill(*simTracksHandle,*simVerticesHandle); | ||
|
|
||
|
|
||
| //SimTrack test; |
| for(size_t i=0;i<allselectedgentracks.size();i++){ | ||
| auto track=allselectedgentracks.at(i); | ||
| if(track->noMother())continue; | ||
| for(size_t j=0;j<allselectedgentracks.size();j++){ |
There was a problem hiding this comment.
Can't we do the usual half matrix instead of the full N^2 loop?, Like starting j=i+1?
There was a problem hiding this comment.
The problem is that 'i' can be mother of 'j', but 'j' not mother of 'i'. We could ask the cross reference, that is true, then we could use the half matrix.
There was a problem hiding this comment.
@rovere Let me know if you're OK with leaving it like that or prefer the cross reference.
There was a problem hiding this comment.
@jkiesele maybe running on half the matrix and testing both cases (i is a mother for j and j is a mother for i) would cover all possibilities, moving the ifs inside the nested loops.
There was a problem hiding this comment.
Ok, I will implement it.
Can we move forward after I implemented it?
It is already in the pipeline for quite some time.
There was a problem hiding this comment.
Sure, thanks, I had hoped that Luca @malgeri would comment on the other item. Maybe I'll catch him tomorrow.
|
Let's briefly discuss on Monday morning at our meeting whether to keep the |
|
Waiting for changes in CMS-HGCAL/reco-prodtools#33 and will then merge. Discussion with Luca gave that we do not need to retain |
add anti-particles
Addresses #45 .
Changes proposed in this pull-request:
Additions:
A1) extrapolation of each generated particle to the first HGCal layer (whether it decayed or not). Corresponding 4 branches (exx,exy,exeta,exphi) are added to the ntuple. Needed for DNN studies.
A2) origin vertex information (x,y,z), there used to be only the decay vertex information
Bugfixes:
B1) change all extrapolations to FullSim rather than FastSim. FastSim does not do a good job in the inhomogeneous field in the endcaps
Changes:
C1) Right now, each electron or particle that fulfils "myTrack.genpartIndex()>=0" gets fully propagated to the HGCal and it's decay vertex is set to the first HGCal layer, even if it decays in the meantime. This overlaps with the additions in A1 and was always affected by bug B1. Please let me know if this feature is used by anybody. Since the same information is now given by addition A1, I would propose to remove this feature.
For the DNN studies, I need the information on the real decay vertex, that I would otherwise need to add for these particular particles in new branches.