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

Better TrajectorySeed interface to recHits #29971

Merged
merged 4 commits into from Jun 5, 2020
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
12 changes: 4 additions & 8 deletions DPGAnalysis/SiStripTools/plugins/SeedMultiplicityAnalyzer.cc
Expand Up @@ -372,7 +372,7 @@ void SeedMultiplicityAnalyzer::analyze(const edm::Event& iEvent, const edm::Even
if (filter->isSelected(iseed)) {
++nseeds;

TransientTrackingRecHit::RecHitPointer recHit = theTTRHBuilder->build(&*(seed->recHits().second - 1));
TransientTrackingRecHit::RecHitPointer recHit = theTTRHBuilder->build(&*(seed->recHits().end() - 1));
TrajectoryStateOnSurface state =
trajectoryStateTransform::transientState(seed->startingState(), recHit->surface(), theMF.product());
// TrajectoryStateClosestToBeamLine tsAtClosestApproachSeed = tscblBuilder(*state.freeState(),bs); // here I need them BS
Expand All @@ -388,17 +388,13 @@ void SeedMultiplicityAnalyzer::analyze(const edm::Event& iEvent, const edm::Even
(*histoeta2D)[i]->Fill(tmpmult[i], eta);
}

// loop on rec hits

TrajectorySeed::range rechits = seed->recHits();

int npixelrh = 0;
for (TrajectorySeed::const_iterator hit = rechits.first; hit != rechits.second; ++hit) {
const SiPixelRecHit* sphit = dynamic_cast<const SiPixelRecHit*>(&(*hit));
for (auto const& hit : seed->recHits()) {
const SiPixelRecHit* sphit = dynamic_cast<const SiPixelRecHit*>(&hit);
if (sphit) {
++npixelrh;
// compute state on recHit surface
TransientTrackingRecHit::RecHitPointer ttrhit = theTTRHBuilder->build(&*hit);
TransientTrackingRecHit::RecHitPointer ttrhit = theTTRHBuilder->build(&hit);
TrajectoryStateOnSurface tsos =
trajectoryStateTransform::transientState(seed->startingState(), ttrhit->surface(), theMF.product());

Expand Down
2 changes: 1 addition & 1 deletion DQM/TrackingMonitor/src/TrackBuildingAnalyzer.cc
Expand Up @@ -525,7 +525,7 @@ void TrackBuildingAnalyzer::analyze(const edm::Event& iEvent,
//double qoverp = tsAtClosestApproachSeed.trackStateAtPCA().charge()/p.mag();
//double theta = p.theta();
//double lambda = M_PI/2-p.theta();
double numberOfHits = candidate.recHits().second - candidate.recHits().first;
double numberOfHits = candidate.nHits();
double dxy = (-v.x() * sin(p.phi()) + v.y() * cos(p.phi()));
double dz = v.z() - (v.x() * p.x() + v.y() * p.y()) / p.perp() * p.z() / p.perp();

Expand Down
3 changes: 1 addition & 2 deletions DataFormats/EgammaReco/interface/ElectronSeed.h
Expand Up @@ -65,7 +65,6 @@ namespace reco {
void setDet(int iDetId, int iLayerOrDiskNr);
};

typedef edm::OwnVector<TrackingRecHit> RecHitContainer;
typedef edm::RefToBase<CaloCluster> CaloClusterRef;
typedef edm::Ref<TrackCollection> CtfTrackRef;

Expand Down Expand Up @@ -135,7 +134,7 @@ namespace reco {
const float dRZ2Pos,
const float dRZ2Neg,
const char hitMask,
const TrajectorySeed::range recHits);
TrajectorySeed::RecHitRange const& recHits);

private:
static float bestVal(float val1, float val2) { return std::abs(val1) < std::abs(val2) ? val1 : val2; }
Expand Down
14 changes: 7 additions & 7 deletions DataFormats/EgammaReco/src/ElectronSeed.cc
Expand Up @@ -25,7 +25,7 @@ ElectronSeed::ElectronSeed(const TrajectorySeed& seed)
isEcalDriven_(false),
isTrackerDriven_(false) {}

ElectronSeed::ElectronSeed(PTrajectoryStateOnDet& pts, recHitContainer& rh, PropagationDirection& dir)
ElectronSeed::ElectronSeed(PTrajectoryStateOnDet& pts, RecHitContainer& rh, PropagationDirection& dir)
: TrajectorySeed(pts, rh, dir),
ctfTrack_(),
caloCluster_(),
Expand All @@ -48,7 +48,7 @@ unsigned int ElectronSeed::hitsMask() const {
int mask = 0;
for (size_t hitNr = 0; hitNr < nHits(); hitNr++) {
int bitNr = 0x1 << hitNr;
int hitDetId = (recHits().first + hitNr)->geographicalId().rawId();
int hitDetId = (recHits().begin() + hitNr)->geographicalId().rawId();
auto detIdMatcher = [hitDetId](const ElectronSeed::PMVars& var) { return hitDetId == var.detId; };
if (std::find_if(hitInfo_.begin(), hitInfo_.end(), detIdMatcher) != hitInfo_.end()) {
mask |= bitNr;
Expand Down Expand Up @@ -79,7 +79,7 @@ void ElectronSeed::initTwoHitSeed(const unsigned char hitMask) {
auto& info = hitInfo_[hitNr];
info.setDPhi(std::numeric_limits<float>::infinity(), std::numeric_limits<float>::infinity());
info.setDRZ(std::numeric_limits<float>::infinity(), std::numeric_limits<float>::infinity());
info.setDet((recHits().first + hitNrs[hitNr])->geographicalId(), -1);
info.setDet((recHits().begin() + hitNrs[hitNr])->geographicalId(), -1);
}
}

Expand Down Expand Up @@ -128,11 +128,11 @@ std::vector<ElectronSeed::PMVars> ElectronSeed::createHitInfo(const float dPhi1P
const float dRZ2Pos,
const float dRZ2Neg,
const char hitMask,
const TrajectorySeed::range recHits) {
TrajectorySeed::RecHitRange const& recHits) {
if (hitMask == 0)
return std::vector<ElectronSeed::PMVars>(); //was trackerDriven so no matched hits

size_t nrRecHits = std::distance(recHits.first, recHits.second);
size_t nrRecHits = std::distance(recHits.begin(), recHits.end());
std::vector<unsigned int> hitNrs = hitNrsFromMask(hitMask);

if (hitNrs.size() != 2) {
Expand All @@ -153,12 +153,12 @@ std::vector<ElectronSeed::PMVars> ElectronSeed::createHitInfo(const float dPhi1P
hitInfo[0].setDPhi(dPhi1Pos, dPhi1Neg);
hitInfo[0].setDRZ(dRZ1Pos, dRZ1Neg);
hitInfo[0].setDet(
(recHits.first + hitNrs[0])->geographicalId(),
(recHits.begin() + hitNrs[0])->geographicalId(),
-1); //getting the layer information needs tracker topo, hence why its stored in the first as its a pain to access
hitInfo[1].setDPhi(dPhi2Pos, dPhi2Neg);
hitInfo[1].setDRZ(dRZ2Pos, dRZ2Neg);
hitInfo[1].setDet(
(recHits.first + hitNrs[1])->geographicalId(),
(recHits.begin() + hitNrs[1])->geographicalId(),
-1); //getting the layer information needs tracker topo, hence why its stored in the first as its a pain to access
return hitInfo;
}
Expand Down
2 changes: 0 additions & 2 deletions DataFormats/MuonSeed/interface/L2MuonTrajectorySeed.h
Expand Up @@ -16,8 +16,6 @@

class L2MuonTrajectorySeed : public TrajectorySeed {
public:
typedef edm::OwnVector<TrackingRecHit> RecHitContainer;

/// Default constructor
L2MuonTrajectorySeed();

Expand Down
2 changes: 0 additions & 2 deletions DataFormats/MuonSeed/interface/L3MuonTrajectorySeed.h
Expand Up @@ -15,8 +15,6 @@

class L3MuonTrajectorySeed : public TrajectorySeed {
public:
typedef edm::OwnVector<TrackingRecHit> RecHitContainer;

/// Default constructor
L3MuonTrajectorySeed() {}

Expand Down
4 changes: 2 additions & 2 deletions DataFormats/MuonSeed/src/L2MuonTrajectorySeed.cc
Expand Up @@ -12,15 +12,15 @@ L2MuonTrajectorySeed::L2MuonTrajectorySeed() : TrajectorySeed() {}

// Constructor
L2MuonTrajectorySeed::L2MuonTrajectorySeed(PTrajectoryStateOnDet const& ptsos,
recHitContainer const& rh,
RecHitContainer const& rh,
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid ambiguities, shouldn't the definition of RecHitContainer in

typedef edm::OwnVector<TrackingRecHit> RecHitContainer;

be removed, so that the one from "DataFormats/TrajectorySeed/interface/TrajectorySeed.h" takes over?

Copy link
Contributor

Choose a reason for hiding this comment

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

If that was your intention, then the re-definition of RecHitContainer should be also removed from

  • DataFormats/​MuonSeed/​interface/​L3MuonTrajectorySeed.h
  • DataFormats/TrackCandidate/interface/TrackCandidate.h

(if I interpret correctly your purpose, as this is what you did in ElectronSeed)

Copy link
Contributor

Choose a reason for hiding this comment

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

Then perhaps TrajectorySeed::RecHitContainer explicitely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was my intention. Thanks for pointing out these other typedefs, I'll remove them as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, I just realized that TrackCandidate does not derive from TrajectorySeed. In that case I think it makes sense that it keeps it's own typedef for the rechit collection. But for the TrajectorySeed derived classes I remove it for sure!

PropagationDirection dir,
l1extra::L1MuonParticleRef l1Ref)
: TrajectorySeed(ptsos, rh, dir) {
theL1Particle = l1Ref;
}

L2MuonTrajectorySeed::L2MuonTrajectorySeed(PTrajectoryStateOnDet const& ptsos,
recHitContainer const& rh,
RecHitContainer const& rh,
PropagationDirection dir,
l1t::MuonRef l1Ref)
: TrajectorySeed(ptsos, rh, dir) {
Expand Down
2 changes: 0 additions & 2 deletions DataFormats/ParticleFlowReco/interface/ConvBremSeed.h
Expand Up @@ -23,8 +23,6 @@ namespace reco {

class ConvBremSeed : public TrajectorySeed {
public:
typedef edm::OwnVector<TrackingRecHit> recHitContainer;

ConvBremSeed() {}
~ConvBremSeed() override {}

Expand Down
16 changes: 8 additions & 8 deletions DataFormats/TrajectorySeed/interface/TrajectorySeed.h
Expand Up @@ -5,6 +5,7 @@
#include "DataFormats/Common/interface/OwnVector.h"
#include "DataFormats/TrackingRecHit/interface/TrackingRecHit.h"
#include "DataFormats/TrajectoryState/interface/PTrajectoryStateOnDet.h"
#include "FWCore/Utilities/interface/Range.h"
#include <utility>
#include <algorithm>

Expand All @@ -16,20 +17,19 @@
**/
class TrajectorySeed {
public:
typedef edm::OwnVector<TrackingRecHit> recHitContainer;
typedef recHitContainer::const_iterator const_iterator;
typedef std::pair<const_iterator, const_iterator> range;
typedef edm::OwnVector<TrackingRecHit> RecHitContainer;
typedef edm::Range<RecHitContainer::const_iterator> RecHitRange;

TrajectorySeed() {}
virtual ~TrajectorySeed() {}

TrajectorySeed(PTrajectoryStateOnDet const& ptsos, recHitContainer const& rh, PropagationDirection dir)
TrajectorySeed(PTrajectoryStateOnDet const& ptsos, RecHitContainer const& rh, PropagationDirection dir)
: hits_(rh), tsos_(ptsos), dir_(dir) {}

TrajectorySeed(PTrajectoryStateOnDet const& ptsos, recHitContainer&& rh, PropagationDirection dir) noexcept
TrajectorySeed(PTrajectoryStateOnDet const& ptsos, RecHitContainer&& rh, PropagationDirection dir) noexcept
: hits_(std::move(rh)), tsos_(ptsos), dir_(dir) {}

void swap(PTrajectoryStateOnDet& ptsos, recHitContainer& rh, PropagationDirection& dir) noexcept {
void swap(PTrajectoryStateOnDet& ptsos, RecHitContainer& rh, PropagationDirection& dir) noexcept {
hits_.swap(rh);
std::swap(tsos_, ptsos);
std::swap(dir_, dir);
Expand All @@ -49,15 +49,15 @@ class TrajectorySeed {

TrajectorySeed& operator=(TrajectorySeed&& o) noexcept = default;

range recHits() const { return std::make_pair(hits_.begin(), hits_.end()); }
RecHitRange recHits() const { return {hits_.begin(), hits_.end()}; }
unsigned int nHits() const { return hits_.size(); }
PropagationDirection direction() const { return dir_; }
PTrajectoryStateOnDet const& startingState() const { return tsos_; }

virtual TrajectorySeed* clone() const { return new TrajectorySeed(*this); }

private:
edm::OwnVector<TrackingRecHit> hits_;
RecHitContainer hits_;
PTrajectoryStateOnDet tsos_;
PropagationDirection dir_;
};
Expand Down
2 changes: 1 addition & 1 deletion DataFormats/TrajectorySeed/test/TrajectorySeed_t.cpp
Expand Up @@ -7,7 +7,7 @@ int main() {
typedef std::vector<TrajectorySeed> TV;

// absurd: just for test
TrajectorySeed::recHitContainer c;
TrajectorySeed::RecHitContainer c;
c.reserve(1000);
for (int i = 0; i != 100; ++i)
c.push_back(new SiStripMatchedRecHit2D);
Expand Down
4 changes: 1 addition & 3 deletions FastSimulation/Muons/plugins/FastTSGFromIOHit.cc
Expand Up @@ -60,9 +60,7 @@ void FastTSGFromIOHit::trackerSeeds(const TrackCand& staMuon,
for (const auto& seeds : seedCollections) {
for (const auto& seed : *seeds) {
// Find the simtrack corresponding to the seed
TrajectorySeed::range recHitRange = seed.recHits();
const FastTrackerRecHit* firstRecHit = (const FastTrackerRecHit*)(&(*(recHitRange.first)));
int simTrackId = firstRecHit->simTrackId(0);
int simTrackId = static_cast<FastTrackerRecHit const&>(*seed.recHits().begin()).simTrackId(0);
const SimTrack& simTrack = (*simTracks)[simTrackId];

// skip if simTrack already associated to a seed
Expand Down
8 changes: 2 additions & 6 deletions FastSimulation/Muons/plugins/FastTSGFromL2Muon.cc
Expand Up @@ -103,12 +103,8 @@ void FastTSGFromL2Muon::produce(edm::Event& ev, const edm::EventSetup& es) {
// The seed
const BasicTrajectorySeed* aSeed = &((*aSeedCollection)[seednr]);

// Find the first hit of the Seed
TrajectorySeed::range theSeedingRecHitRange = aSeed->recHits();
const FastTrackerRecHit* theFirstSeedingRecHit = (const FastTrackerRecHit*)(&(*(theSeedingRecHitRange.first)));

// The SimTrack id associated to that recHit
int simTrackId = theFirstSeedingRecHit->simTrackId(0);
// The SimTrack id associated to the first hit of the Seed
int simTrackId = static_cast<FastTrackerRecHit const&>(*aSeed->recHits().begin()).simTrackId(0);

// Track already associated to a seed
std::set<unsigned>::iterator tkId = tkIds.find(simTrackId);
Expand Down
4 changes: 2 additions & 2 deletions FastSimulation/Tracking/interface/FastTrackingUtilities.h
Expand Up @@ -21,12 +21,12 @@ namespace fastTrackingUtilities {
template <class T>
int32_t getRecHitCombinationIndex(const T &object) {
// seed must have at least one hit
if (object.recHits().first == object.recHits().second) {
if (object.recHits().empty()) {
throw cms::Exception("fastTrackingHelpers::getRecHitCombinationIndex")
<< " given object has 0 hits" << std::endl;
}

const TrackingRecHit &recHit = *object.recHits().first;
const TrackingRecHit &recHit = *object.recHits().begin();
if (!trackerHitRTTI::isFast(recHit)) {
throw cms::Exception("fastTrackingHelpers::setRecHitCombinationIndex")
<< " one of hits in OwnVector is non-fastsim" << std::endl;
Expand Down
12 changes: 5 additions & 7 deletions FastSimulation/Tracking/plugins/PixelTracksProducer.cc
Expand Up @@ -98,15 +98,13 @@ void PixelTracksProducer::produce(edm::Event& e, const edm::EventSetup& es) {
TrajectorySeedCollection::const_iterator aSeed = theSeeds->begin();
TrajectorySeedCollection::const_iterator lastSeed = theSeeds->end();
for (; aSeed != lastSeed; ++aSeed) {
// Find the first hit and last hit of the Seed
TrajectorySeed::range theSeedingRecHitRange = aSeed->recHits();
edm::OwnVector<TrackingRecHit>::const_iterator aSeedingRecHit = theSeedingRecHitRange.first;
edm::OwnVector<TrackingRecHit>::const_iterator theLastSeedingRecHit = theSeedingRecHitRange.second;

// Loop over the rechits
std::vector<const TrackingRecHit*> TripletHits(3, static_cast<const TrackingRecHit*>(nullptr));
for (unsigned i = 0; aSeedingRecHit != theLastSeedingRecHit; ++i, ++aSeedingRecHit)
TripletHits[i] = &(*aSeedingRecHit);
unsigned int iRecHit = 0;
for (auto const& recHit : aSeed->recHits()) {
TripletHits[iRecHit] = &recHit;
++iRecHit;
}

// fitting the triplet
std::unique_ptr<reco::Track> track = fitter.run(TripletHits, region, es);
Expand Down
5 changes: 2 additions & 3 deletions FastSimulation/Tracking/plugins/TrackCandidateProducer.cc
Expand Up @@ -140,9 +140,8 @@ void TrackCandidateProducer::produce(edm::Event& e, const edm::EventSetup& es) {
std::vector<const FastTrackerRecHit*> selectedRecHits;

// add the seed hits
TrajectorySeed::range seedHitRange = seed.recHits(); //Hits in a seed
for (TrajectorySeed::const_iterator ihit = seedHitRange.first; ihit != seedHitRange.second; ++ihit) {
selectedRecHits.push_back(static_cast<const FastTrackerRecHit*>(&*ihit));
for (auto const& recHit : seed.recHits()) {
selectedRecHits.push_back(static_cast<const FastTrackerRecHit*>(&recHit));
}

// prepare to skip seed hits
Expand Down
1 change: 0 additions & 1 deletion FastSimulation/Tracking/test/testGeneralTracks.cc
Expand Up @@ -174,7 +174,6 @@ void testGeneralTracks::analyze(const edm::Event& iEvent, const edm::EventSetup&

std::vector<bool> firstSeed(2, static_cast<bool>(false));
std::vector<bool> secondSeed(2, static_cast<bool>(false));
std::vector<TrajectorySeed::range> theRecHitRange(2);

for (unsigned ievt = 0; ievt < 2; ++ievt) {
edm::Handle<reco::TrackCollection> tkRef0;
Expand Down
9 changes: 4 additions & 5 deletions Fireworks/Tracks/plugins/FWTrajectorySeedProxyBuilder.cc
Expand Up @@ -60,13 +60,12 @@ void FWTrajectorySeedProxyBuilder::build(const TrajectorySeed& iData,
TEvePointSet* pointSet = new TEvePointSet;
TEveLine* line = new TEveLine;
TEveStraightLineSet* lineSet = new TEveStraightLineSet;
TrajectorySeed::const_iterator hit = iData.recHits().first;

for (; hit != iData.recHits().second; hit++) {
unsigned int id = hit->geographicalId();
for (auto const& hit : iData.recHits()) {
unsigned int id = hit.geographicalId();
const FWGeometry* geom = item()->getGeom();
const float* pars = geom->getParameters(id);
const SiPixelRecHit* rh = dynamic_cast<const SiPixelRecHit*>(&*hit);
const SiPixelRecHit* rh = dynamic_cast<const SiPixelRecHit*>(&hit);
// std::cout << id << "id "<< std::endl;
if (rh) {
const SiPixelCluster* itc = rh->cluster().get();
Expand All @@ -87,7 +86,7 @@ void FWTrajectorySeedProxyBuilder::build(const TrajectorySeed& iData,
}

else {
const SiStripCluster* cluster = fireworks::extractClusterFromTrackingRecHit(&*hit);
const SiStripCluster* cluster = fireworks::extractClusterFromTrackingRecHit(&hit);

if (cluster) {
short firststrip = cluster->firstStrip();
Expand Down
7 changes: 3 additions & 4 deletions HLTrigger/Muon/plugins/HLTMuonTrackMassFilter.cc
Expand Up @@ -350,10 +350,9 @@ bool HLTMuonTrackMassFilter::pairMatched(std::vector<reco::RecoChargedCandidateR
// comparison by hits of TrajectorySeed of the new track
// with the previous candidate
//
TrajectorySeed::range seedHits = seedRef->recHits();
const TrajectorySeed::RecHitRange seedHits = seedRef->recHits();
trackingRecHit_iterator prevTrackHitEnd;
trackingRecHit_iterator iprev;
TrajectorySeed::const_iterator iseed;
for (size_t i = 0; i < prevMuonRefs.size(); ++i) {
// identity of muon
if (prevMuonRefs[i] != muonRef)
Expand All @@ -369,11 +368,11 @@ bool HLTMuonTrackMassFilter::pairMatched(std::vector<reco::RecoChargedCandidateR
if (seedRef->nHits() != prevTrackRef->recHitsSize())
continue;
// hit-by-hit comparison based on the sharesInput method
iseed = seedHits.first;
auto iseed = seedHits.begin();
iprev = prevTrackRef->recHitsBegin();
prevTrackHitEnd = prevTrackRef->recHitsEnd();
bool identical(true);
for (; iseed != seedHits.second && iprev != prevTrackHitEnd; ++iseed, ++iprev) {
for (; iseed != seedHits.end() && iprev != prevTrackHitEnd; ++iseed, ++iprev) {
if ((*iseed).isValid() != (**iprev).isValid() || !(*iseed).sharesInput(&**iprev, TrackingRecHit::all)) {
// terminate loop over hits on first mismatch
identical = false;
Expand Down
7 changes: 3 additions & 4 deletions RecoEgamma/EgammaElectronAlgos/src/ElectronSeedGenerator.cc
Expand Up @@ -37,10 +37,9 @@ namespace {
if (s1.nHits() != s2.nHits())
return false;

unsigned int nHits;
TrajectorySeed::range r1 = s1.recHits(), r2 = s2.recHits();
TrajectorySeed::const_iterator i1, i2;
for (i1 = r1.first, i2 = r2.first, nHits = 0; i1 != r1.second; ++i1, ++i2, ++nHits) {
const TrajectorySeed::RecHitRange r1 = s1.recHits();
const TrajectorySeed::RecHitRange r2 = s2.recHits();
for (auto i1 = r1.begin(), i2 = r2.begin(); i1 != r1.end(); ++i1, ++i2) {
if (!i1->isValid() || !i2->isValid())
return false;
if (i1->geographicalId() != i2->geographicalId())
Expand Down