Skip to content
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

New TrackingRecHitRange and use it in RecoParticleFlow #25591

Merged
merged 3 commits into from Jan 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions DataFormats/CSCRecHit/test/CSCSharesInputTest.cc
Expand Up @@ -67,13 +67,13 @@ void CSCSharesInputTest::analyze(const edm::Event &myEvent, const edm::EventSetu
perEventData[2] += track->recHitsSize();
perMuonData[0] += track->recHitsSize();

for (trackingRecHit_iterator jHit = track->recHitsBegin(); jHit != track->recHitsEnd(); ++jHit) {
for(auto const& jHit : track->recHits()) {

// AllMatched:SomeMatched:AllWiresMatched:SomeWiresMatched:AllStripsMatched:SomeStripsMatched:NotMatched
float perRecHitData[7] = {0,0,0,0,0,0,0};

// Kill us quickly if this is not a CSCRecHit. Also allows us to use the CSCRecHit version of sharesInput, which we like.
const CSCRecHit2D *myHit = dynamic_cast<const CSCRecHit2D *>((*jHit));
const CSCRecHit2D *myHit = dynamic_cast<const CSCRecHit2D *>(jHit);
if (myHit == 0) {
++counts_["NotMatchedRecHits"];
++perEventData[9];
Expand Down
3 changes: 3 additions & 0 deletions DataFormats/TrackReco/interface/Track.h
Expand Up @@ -101,6 +101,9 @@ class Track : public TrackBase
unsigned int innerDetId() const {
return extra_->innerDetId();
}

/// Access to reconstructed hits on the track.
auto recHits() const { return extra_->recHits(); }

/// Iterator to first hit on the track.
trackingRecHit_iterator recHitsBegin() const {
Expand Down
2 changes: 2 additions & 0 deletions DataFormats/TrackReco/interface/TrackExtraBase.h
Expand Up @@ -44,6 +44,8 @@ class TrackExtraBase
return m_nHits;
}

/// accessor to RecHits
auto recHits() const { return TrackingRecHitRange(recHitsBegin(), recHitsEnd()); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, you could just use recHitsProduct() that returns const reference to the actual container.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not the same: https://github.com/cms-sw/cmssw/blob/master/DataFormats/TrackReco/interface/TrackExtraBase.h#L50

Begin would be recHitsProduct().data().begin()+firstRecHit(); and end recHitsBegin()+recHitsSize();. So it seems to me that the desired range is just a part (sub-range of elements next to each other) of the original product. Unless m_firstHit and m_nHits would always be zero and the size of the collection of course, I have not checked that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it's not exactly the same: recHitsProduct() is edm::OwnVector<T> and recHitsProduct().data() is std::vector<T *>. But OwnVector has also begin()+end() and iterators defined, so it should work with range-for (even if OwnVector::(const_)iterator is different type than trackingRecHit_iterator).

Copy link
Contributor Author

@guitargeek guitargeek Jan 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so we can leave out the data() call. But in the end it's not about the data(), it's about the +firstRecHit(), right? Iterating over OwnVector would iterate over the same elements as iterating over the base vector I think, not the window that is defined by firstRecHit() and recHitsSize(). So the little helper class is still needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, you're right, I completely missed +firstRecHit() (and forgot how the RecHits are organized within TrackExtra, shame on me). That indeed implies recHitsProduct() does not work, and one does need a range wrapper to be able to use range-for.


/// first iterator over RecHits
trackingRecHit_iterator recHitsBegin() const {
Expand Down
3 changes: 3 additions & 0 deletions DataFormats/TrackingRecHit/interface/TrackingRecHitFwd.h
Expand Up @@ -5,6 +5,7 @@
#include "DataFormats/Common/interface/RefProd.h"
#include "DataFormats/Common/interface/RefVector.h"
#include "DataFormats/Common/interface/OwnVector.h"
#include "FWCore/Utilities/interface/Range.h"

class TrackingRecHit;
/// collection of TrackingRecHits
Expand All @@ -17,5 +18,7 @@ typedef edm::RefProd<TrackingRecHitCollection> TrackingRecHitRefProd;
typedef edm::RefVector<TrackingRecHitCollection> TrackingRecHitRefVector;
/// iterator over a vector of reference to TrackingRecHit in the same collection
typedef TrackingRecHitCollection::base::const_iterator trackingRecHit_iterator;
/// Range class to enable range-based loops for a tracks RecHits
using TrackingRecHitRange = edm::Range<trackingRecHit_iterator>;

#endif
26 changes: 26 additions & 0 deletions FWCore/Utilities/interface/Range.h
@@ -0,0 +1,26 @@
#ifndef FWCore_Utilities_Range_h
#define FWCore_Utilities_Range_h

namespace edm
{
/*
*class which implements begin() and end() to use range-based loop with
pairs of iterators or pointers.
*/

template<class T>
class Range
{
public:
Range(T begin, T end) : begin_(begin), end_(end) {}

T begin() const { return begin_; }
T end() const { return end_; }

private:
const T begin_;
const T end_;
};
};

#endif
48 changes: 24 additions & 24 deletions RecoMuon/TrackingTools/src/MuonSegmentMatcher.cc
Expand Up @@ -76,13 +76,13 @@ vector<const DTRecSegment4D*> MuonSegmentMatcher::matchDT(const reco::Track &muo
bool segments = false;

// Loop and select DT recHits
for(trackingRecHit_iterator hit = muon.recHitsBegin(); hit != muon.recHitsEnd(); ++hit) {
if (!(*hit)->isValid()) continue;
if ( (*hit)->geographicalId().det() != DetId::Muon ) continue;
if ( (*hit)->geographicalId().subdetId() != MuonSubdetId::DT ) continue;
if (!(*hit)->recHits().empty())
if ((*(*hit)->recHits().begin())->recHits().size()>1) segments = true;
dtHits.push_back(*hit);
for(auto const& hit : muon.recHits()) {
if (!hit->isValid()) continue;
if ( hit->geographicalId().det() != DetId::Muon ) continue;
if ( hit->geographicalId().subdetId() != MuonSubdetId::DT ) continue;
if (!hit->recHits().empty())
if ((*hit->recHits().begin())->recHits().size()>1) segments = true;
dtHits.push_back(hit);
}

// cout << "Muon DT hits found: " << dtHits.size() << " segments " << segments << endl;
Expand Down Expand Up @@ -258,22 +258,22 @@ vector<const CSCSegment*> MuonSegmentMatcher::matchCSC(const reco::Track& muon,

bool segments = false;

for(trackingRecHit_iterator hitC = muon.recHitsBegin(); hitC != muon.recHitsEnd(); ++hitC) {
if (!(*hitC)->isValid()) continue;
if ( (*hitC)->geographicalId().det() != DetId::Muon ) continue;
if ( (*hitC)->geographicalId().subdetId() != MuonSubdetId::CSC ) continue;
if (!(*hitC)->isValid()) continue;
if ( (*hitC)->recHits().size()>1) segments = true;
for(auto const& hitC : muon.recHits()) {
if (!hitC->isValid()) continue;
if ( hitC->geographicalId().det() != DetId::Muon ) continue;
if ( hitC->geographicalId().subdetId() != MuonSubdetId::CSC ) continue;
if (!hitC->isValid()) continue;
if ( hitC->recHits().size()>1) segments = true;

//DETECTOR CONSTRUCTION
DetId id = (*hitC)->geographicalId();
DetId id = hitC->geographicalId();
CSCDetId cscDetIdHit(id.rawId());

if (segments) {
if(!(myChamber.rawId()==cscDetIdHit.rawId())) continue;

// and compare the local positions
LocalPoint positionLocalCSC = (*hitC)->localPosition();
LocalPoint positionLocalCSC = hitC->localPosition();
LocalPoint segLocalCSC = segmentCSC->localPosition();
if ((fabs(positionLocalCSC.x()-segLocalCSC.x())<CSCXCut) &&
(fabs(positionLocalCSC.y()-segLocalCSC.y())<CSCYCut))
Expand All @@ -288,14 +288,14 @@ vector<const CSCSegment*> MuonSegmentMatcher::matchCSC(const reco::Track& muon,

countMuonCSCHits++;

LocalPoint positionLocalCSC = (*hitC)->localPosition();
LocalPoint positionLocalCSC = hitC->localPosition();

for (vector<CSCRecHit2D>::const_iterator hiti=CSCRechits2D.begin(); hiti!=CSCRechits2D.end(); hiti++) {

if ( !hiti->isValid()) continue;
CSCDetId cscDetId((hiti->geographicalId()).rawId());

if ((*hitC)->geographicalId().rawId()!=(hiti->geographicalId()).rawId()) continue;
if (hitC->geographicalId().rawId()!=(hiti->geographicalId()).rawId()) continue;

LocalPoint segLocalCSC = hiti->localPosition();
// cout<<"Layer Id (MuonHit) = "<<cscDetIdHit<<" Muon Local Position (det frame) "<<positionLocalCSC <<endl;
Expand Down Expand Up @@ -338,18 +338,18 @@ vector<const RPCRecHit*> MuonSegmentMatcher::matchRPC(const reco::Track& muon, c
LocalPoint posLocalRPC = hitRPC->localPosition();
bool matched=false;

for(trackingRecHit_iterator hitC = muon.recHitsBegin(); hitC != muon.recHitsEnd(); ++hitC) {
if (!(*hitC)->isValid()) continue;
if ( (*hitC)->geographicalId().det() != DetId::Muon ) continue;
if ( (*hitC)->geographicalId().subdetId() != MuonSubdetId::RPC ) continue;
if (!(*hitC)->isValid()) continue;
for(auto const& hitC : muon.recHits()) {
if (!hitC->isValid()) continue;
if ( hitC->geographicalId().det() != DetId::Muon ) continue;
if ( hitC->geographicalId().subdetId() != MuonSubdetId::RPC ) continue;
if (!hitC->isValid()) continue;

//DETECTOR CONSTRUCTION
DetId id = (*hitC)->geographicalId();
DetId id = hitC->geographicalId();
RPCDetId rpcDetIdHit(id.rawId());

if (rpcDetIdHit!=myChamber) continue;
LocalPoint posLocalMuon = (*hitC)->localPosition();
LocalPoint posLocalMuon = hitC->localPosition();

// cout<<"Layer Id (MuonHit) = "<<rpcDetIdHit<<" Muon Local Position (det frame) "<<posLocalMuon <<endl;
// cout<<"Layer Id (RPCHit) = "<<myChamber<<" Hit Local Position (det frame) "<<posLocalRPC <<endl;
Expand Down
10 changes: 5 additions & 5 deletions RecoMuon/TrackingTools/src/SegmentsTrackAssociator.cc
Expand Up @@ -76,22 +76,22 @@ MuonTransientTrackingRecHit::MuonRecHitContainer SegmentsTrackAssociator::associ
MuonTransientTrackingRecHit::MuonRecHitContainer selectedCscSegments;

//loop over recHit
for(trackingRecHit_iterator recHit = Track.recHitsBegin(); recHit != Track.recHitsEnd(); ++recHit){
for(auto const& recHit : Track.recHits()) {

if(!(*recHit)->isValid()) continue;
if(!recHit->isValid()) continue;


numRecHit++;

//get the detector Id
DetId idRivHit = (*recHit)->geographicalId();
DetId idRivHit = recHit->geographicalId();


// DT recHits
if (idRivHit.det() == DetId::Muon && idRivHit.subdetId() == MuonSubdetId::DT ) {

// get the RecHit Local Position
LocalPoint posTrackRecHit = (*recHit)->localPosition();
LocalPoint posTrackRecHit = recHit->localPosition();

DTRecSegment4DCollection::range range;
numRecHitDT++;
Expand Down Expand Up @@ -160,7 +160,7 @@ MuonTransientTrackingRecHit::MuonRecHitContainer SegmentsTrackAssociator::associ
if (idRivHit.det() == DetId::Muon && idRivHit.subdetId() == MuonSubdetId::CSC ) {

// get the RecHit Local Position
LocalPoint posTrackRecHit = (*recHit)->localPosition();
LocalPoint posTrackRecHit = recHit->localPosition();

CSCSegmentCollection::range range;
numRecHitCSC++;
Expand Down
11 changes: 3 additions & 8 deletions RecoParticleFlow/PFSimProducer/plugins/ConvBremSeedProducer.cc
Expand Up @@ -549,15 +549,10 @@ ConvBremSeedProducer::makeTrajectoryState( const DetLayer* layer,
(GlobalTrajectoryParameters( pos, mom, TrackCharge( pp.charge()), field), *plane);
}
bool ConvBremSeedProducer::isGsfTrack(const reco::Track & tkv, const TrackingRecHit *h ){
auto ib=tkv.recHitsBegin();
auto ie=tkv.recHitsEnd();
bool istaken=false;
// for (;ib!=ie-2;++ib){
for (;ib!=ie;++ib){
if (istaken) continue;
if (!((*ib)->isValid())) continue;

istaken = (*ib)->sharesInput(h,TrackingRecHit::all);
for(auto const& hit : tkv.recHits()) {
if (istaken || !hit->isValid()) continue;
istaken = hit->sharesInput(h,TrackingRecHit::all);
}
return istaken;
}
Expand Down
32 changes: 12 additions & 20 deletions RecoParticleFlow/PFSimProducer/plugins/PFSimParticleProducer.cc
Expand Up @@ -490,27 +490,19 @@ void PFSimParticleProducer::getSimIDs( const TrackHandle& trackh,

for(unsigned i=0;i<trackh->size(); i++) {

reco::PFRecTrackRef ref( trackh,i );
const reco::PFRecTrack& PFT = *ref;
const reco::TrackRef& trackref = PFT.trackRef();
const reco::PFRecTrackRef ref( trackh,i );

trackingRecHit_iterator rhitbeg
= trackref->recHitsBegin();
trackingRecHit_iterator rhitend
= trackref->recHitsEnd();
for (trackingRecHit_iterator it = rhitbeg;
it != rhitend; it++){

if( (*it)->isValid() ){

const FastTrackerRecHit * rechit
= (const FastTrackerRecHit*) (*it);

for(unsigned int st_index=0;st_index<rechit->nSimTrackIds();++st_index){
recTrackSimID.push_back(rechit->simTrackId(st_index));
}
break;
}
for(auto const& hit : ref->trackRef()->recHits())
{
if( hit->isValid() ) {

auto rechit = dynamic_cast<const FastTrackerRecHit*>(hit);

for(unsigned int st_index=0;st_index<rechit->nSimTrackIds();++st_index){
recTrackSimID.push_back(rechit->simTrackId(st_index));
}
break;
}
}//loop track rechit
}//loop recTracks
}//track handle valid
Expand Down
5 changes: 1 addition & 4 deletions RecoParticleFlow/PFTracking/plugins/GoodSeedProducer.cc
Expand Up @@ -370,10 +370,7 @@ GoodSeedProducer::produce(Event& iEvent, const EventSetup& iSetup)
trk_ecalDphi = trk_ecalDphi_;

Trajectory::ConstRecHitContainer tmp;
auto hb = Tk[i].recHitsBegin();
for(unsigned int h=0;h<Tk[i].recHitsSize();h++){
auto recHit = *(hb+h); tmp.push_back(recHit->cloneSH());
}
for(auto const& hit : Tk[i].recHits()) tmp.push_back(hit->cloneSH());
auto const & theTrack = Tk[i];
GlobalVector gv(theTrack.innerMomentum().x(),theTrack.innerMomentum().y(),theTrack.innerMomentum().z());
GlobalPoint gp(theTrack.innerPosition().x(),theTrack.innerPosition().y(),theTrack.innerPosition().z());
Expand Down
17 changes: 6 additions & 11 deletions RecoParticleFlow/PFTracking/plugins/PFConversionProducer.cc
Expand Up @@ -98,17 +98,12 @@ PFConversionProducer::produce(Event& iEvent, const EventSetup& iSetup)
double like2=-999;
//number of shared hits
int shared=0;
for(trackingRecHit_iterator iHit1=trackRef1->recHitsBegin(); iHit1!=trackRef1->recHitsEnd(); iHit1++)
{
const TrackingRecHit *h_1=(*iHit1);
if(h_1->isValid()){
for(trackingRecHit_iterator iHit2=trackRef2->recHitsBegin(); iHit2!=trackRef2->recHitsEnd(); iHit2++)
{
const TrackingRecHit *h_2=(*iHit2);
if(h_2->isValid() && h_1->sharesInput(h_2, TrackingRecHit::some))shared++;//count number of shared hits
}
}
}
for(auto const& hit1 : trackRef1->recHits()) if(hit1->isValid()) {
//count number of shared hits
for(auto const& hit2 : trackRef2->recHits()) {
if(hit2->isValid() && hit1->sharesInput(hit2, TrackingRecHit::some)) shared++;
}
}
float frac=0;
//number of valid hits in tracks that are duplicates
float size1=trackRef1->found();
Expand Down
50 changes: 18 additions & 32 deletions RecoParticleFlow/PFTracking/plugins/PFElecTkProducer.cc
Expand Up @@ -421,26 +421,16 @@ PFElecTkProducer::FindPfRef(const reco::PFRecTrackCollection & PfRTkColl,
float det=fabs(pft->trackRef()->eta()-gsftk.eta());
float dr =sqrt(dph*dph+det*det);

trackingRecHit_iterator hhit=
pft->trackRef()->recHitsBegin();
trackingRecHit_iterator hhit_end=
pft->trackRef()->recHitsEnd();



for(;hhit!=hhit_end;++hhit){
if (!(*hhit)->isValid()) continue;
TrajectorySeed::const_iterator hit=
gsftk.seedRef()->recHits().first;
TrajectorySeed::const_iterator hit_end=
gsftk.seedRef()->recHits().second;
for(;hit!=hit_end;++hit){
if (!(hit->isValid())) continue;
if((*hhit)->sharesInput(&*(hit),TrackingRecHit::all)) ish++;
// if((hit->geographicalId()==(*hhit)->geographicalId())&&
// (((*hhit)->localPosition()-hit->localPosition()).mag()<0.01)) ish++;
}

for(auto const& hhit : pft->trackRef()->recHits()) {
if (!hhit->isValid()) continue;
TrajectorySeed::const_iterator hit = gsftk.seedRef()->recHits().first;
TrajectorySeed::const_iterator hit_end = gsftk.seedRef()->recHits().second;
for(;hit!=hit_end;++hit) {
if (!(hit->isValid())) continue;
if(hhit->sharesInput(&*(hit),TrackingRecHit::all)) ish++;
// if((hit->geographicalId()==hhit->geographicalId())&&
// ((hhit->localPosition()-hit->localPosition()).mag()<0.01)) ish++;
}
}


Expand Down Expand Up @@ -1045,20 +1035,16 @@ bool PFElecTkProducer::isInnerMost(const reco::GsfTrackRef& nGsfTrack,

// retrieve first valid hit
int gsfHitCounter1 = 0 ;
trackingRecHit_iterator elHitsIt1 ;
for
( elHitsIt1 = nGsfTrack->recHitsBegin() ;
elHitsIt1 != nGsfTrack->recHitsEnd() ;
elHitsIt1++, gsfHitCounter1++ )
{ if (((**elHitsIt1).isValid())) break ; }
for(auto const& hit : nGsfTrack->recHits()) {
if (hit->isValid()) break ;
gsfHitCounter1++;
}

int gsfHitCounter2 = 0 ;
trackingRecHit_iterator elHitsIt2 ;
for
( elHitsIt2 = iGsfTrack->recHitsBegin() ;
elHitsIt2 != iGsfTrack->recHitsEnd() ;
elHitsIt2++, gsfHitCounter2++ )
{ if (((**elHitsIt2).isValid())) break ; }
for(auto const& hit : iGsfTrack->recHits()) {
if (hit->isValid()) break ;
gsfHitCounter2++;
}

uint32_t gsfHit1 = gsfHitPattern1.getHitPattern(HitPattern::TRACK_HITS, gsfHitCounter1) ;
uint32_t gsfHit2 = gsfHitPattern2.getHitPattern(HitPattern::TRACK_HITS, gsfHitCounter2) ;
Expand Down