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.

Sign up for GitHub

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

Speed seeding for HLT #13819

Merged
merged 13 commits into from
Apr 4, 2016
Merged

Speed seeding for HLT #13819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1ccaa0e
add index for gdets
VinInn Mar 11, 2016
7ab420a
faster, not as much as expected
VinInn Mar 12, 2016
7411c20
infinitely faster hash
VinInn Mar 13, 2016
d770963
cure expensive math
VinInn Mar 13, 2016
50bdcca
ofast
VinInn Mar 13, 2016
6cba9d5
exchange tests: slower second
VinInn Mar 13, 2016
c2a125f
speed up EtaPhiMeasurementEstimator
VinInn Mar 14, 2016
53b0815
modernize EtaPhiRange, remove obsolere GlobalDetRange
VinInn Mar 16, 2016
0e5d640
devirtualize, speed up math
VinInn Mar 17, 2016
a6bab83
make atan2 3bits more precise
VinInn Mar 17, 2016
393a2aa
fix trivial bug, no regression
VinInn Mar 18, 2016
b4aa8ff
dummy out hit etaphi estimator
VinInn Mar 23, 2016
426feb3
remove debug, add comments
VinInn Apr 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions DataFormats/GeometrySurface/src/BoundSpan.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ void BoundSpan::compute(Surface const & plane) {
if (trapezoidalBounds) {
std::array<const float, 4> const & parameters = (*trapezoidalBounds).parameters();

float hbotedge = parameters[0];
float htopedge = parameters[1];
float hapothem = parameters[3];
float thickness = (*trapezoidalBounds).thickness();
auto hbotedge = parameters[0];
auto htopedge = parameters[1];
auto hapothem = parameters[3];
auto thickness = (*trapezoidalBounds).thickness();

corners[0] = plane.toGlobal( LocalPoint( -htopedge, hapothem, thickness/2));
corners[1] = plane.toGlobal( LocalPoint( htopedge, hapothem, thickness/2));
Expand All @@ -29,9 +29,9 @@ void BoundSpan::compute(Surface const & plane) {
corners[7] = plane.toGlobal( LocalPoint( -hbotedge, -hapothem, -thickness/2));

}else if(rectangularBounds) {
float length = rectangularBounds->length();
float width = rectangularBounds->width();
float thickness = (*rectangularBounds).thickness();
auto length = rectangularBounds->length();
auto width = rectangularBounds->width();
auto thickness = (*rectangularBounds).thickness();

corners[0] = plane.toGlobal( LocalPoint( -width/2, -length/2, thickness/2));
corners[1] = plane.toGlobal( LocalPoint( -width/2, +length/2, thickness/2));
Expand All @@ -47,16 +47,23 @@ void BoundSpan::compute(Surface const & plane) {

float phimin = corners[0].barePhi(); float phimax = phimin;
float zmin = corners[0].z(); float zmax = zmin;
float rmin = corners[0].perp2(); float rmax = rmin;
for ( int i = 1; i < 8; i++ ) {
float cPhi = corners[i].barePhi();
auto cPhi = corners[i].barePhi();
if ( Geom::phiLess( cPhi, phimin)) { phimin = cPhi; }
if ( Geom::phiLess( phimax, cPhi)) { phimax = cPhi; }
float z = corners[i].z();
auto z = corners[i].z();
if ( z < zmin) zmin = z;
if ( z > zmax) zmax = z;
if ( z > zmax) zmax = z;
auto r = corners[i].perp2();
if ( r < rmin) rmin = r;
if ( r > rmax) rmax = r;

}
m_zSpan.first = zmin;
m_zSpan.second = zmax;
m_rSpan.first = std::sqrt(rmin);
m_rSpan.second = std::sqrt(rmax);
m_phiSpan.first = phimin;
m_phiSpan.second = phimax;
}
12 changes: 6 additions & 6 deletions DataFormats/GeometryVector/interface/Pi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@

namespace Geom {

inline double pi() {return 3.141592653589793238;}
inline double twoPi() {return 2. *3.141592653589793238;}
inline double halfPi() {return 0.5*3.141592653589793238;}
inline constexpr double pi() {return 3.141592653589793238;}
inline constexpr double twoPi() {return 2. *3.141592653589793238;}
inline constexpr double halfPi() {return 0.5*3.141592653589793238;}
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use M_PI, 2. * M_PI, M_PI_2 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because is like this since orca times...
(somewhere else you will find CLHEP::pi())
I would not mind to go back to old good C macros
(as already done elsewhere)


inline float fpi() {return 3.141592653589793238f;}
inline float ftwoPi() {return 2.f *3.141592653589793238f;}
inline float fhalfPi() {return 0.5f*3.141592653589793238f;}
inline constexpr float fpi() {return 3.141592653589793238f;}
inline constexpr float ftwoPi() {return 2.f *3.141592653589793238f;}
inline constexpr float fhalfPi() {return 0.5f*3.141592653589793238f;}


}
Expand Down
25 changes: 16 additions & 9 deletions Geometry/CommonDetUnit/interface/GeomDet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ class SurfaceDeformation;

class GeomDet {
public:
typedef GeomDetEnumerators::SubDetector SubDetector;
using SubDetector = GeomDetEnumerators::SubDetector;


explicit GeomDet( Plane* plane): thePlane(plane) {}
explicit GeomDet( const ReferenceCountingPointer<Plane>& plane) : thePlane(plane) {}

explicit GeomDet(Plane* plane);

explicit GeomDet(const ReferenceCountingPointer<Plane>& plane);

virtual ~GeomDet();

Expand Down Expand Up @@ -77,7 +79,7 @@ class GeomDet {
DetId geographicalId() const { return m_detId; }

/// Which subdetector
virtual SubDetector subDetector() const;;
virtual SubDetector subDetector() const;

/// is a Unit
virtual bool isLeaf() const { return components().empty();}
Expand All @@ -93,10 +95,14 @@ class GeomDet {
AlignmentPositionError const* alignmentPositionError() const { return theAlignmentPositionError;}


// specific unix index in a given subdetector (such as Tracker)
// specific unit index in a given subdetector (such as Tracker)
int index() const { return m_index;}
void setIndex(int i) { m_index=i;}

// specific geomDet index in a given subdetector (such as Tracker)
int gdetIndex() const { return m_gdetIndex;}
void setGdetIndex(int i) { m_gdetIndex=i;}


virtual const Topology& topology() const;

Expand All @@ -105,7 +111,7 @@ class GeomDet {

/// Return pointer to surface deformation.
/// Defaults to "null" if not reimplemented in the derived classes.
virtual const SurfaceDeformation* surfaceDeformation() const { return 0; }
virtual const SurfaceDeformation* surfaceDeformation() const { return nullptr; }



Expand All @@ -119,9 +125,10 @@ class GeomDet {

ReferenceCountingPointer<Plane> thePlane;
DetId m_detId;
int m_index;
int m_index=-1;
Copy link
Contributor

Choose a reason for hiding this comment

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

@VinInn - it would be nice to clean up the class code a bit: sort public, protected and private members, name the latter either with leading m_* or the*, indent the code consistently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be nice to cleanup all old code in general yes.
takes time and effort.
Let's schedule it for PhaseI (this code is candidate for backport to 800).

int m_gdetIndex=-1;
protected:
AlignmentPositionError* theAlignmentPositionError;
AlignmentPositionError* theAlignmentPositionError=nullptr;

private:

Expand Down Expand Up @@ -158,7 +165,7 @@ class GeomDet {

};

typedef GeomDet GeomDetUnit;
using GeomDetUnit = GeomDet;

#endif

Expand Down
5 changes: 0 additions & 5 deletions Geometry/CommonDetUnit/src/GeomDet.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@
#include "Geometry/CommonDetUnit/interface/ModifiedSurfaceGenerator.h"
#include "DataFormats/TrackingRecHit/interface/AlignmentPositionError.h"

GeomDet::GeomDet( Plane* plane):
thePlane(plane), m_index(-1), theAlignmentPositionError(nullptr) {}

GeomDet::GeomDet( const ReferenceCountingPointer<Plane>& plane) :
thePlane(plane), m_index(-1), theAlignmentPositionError(nullptr) {}

GeomDet::~GeomDet() {delete theAlignmentPositionError;}

Expand Down
16 changes: 8 additions & 8 deletions Geometry/TrackerGeometryBuilder/interface/TrackerGeometry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,16 @@ class TrackerGeometry final : public TrackingGeometry {
Ph2PSS,
Ph2SS
};
virtual ~TrackerGeometry() ;

virtual ~TrackerGeometry() ;

virtual const DetTypeContainer& detTypes() const;
virtual const DetUnitContainer& detUnits() const;
virtual const DetContainer& dets() const;
virtual const DetIdContainer& detUnitIds() const;
virtual const DetIdContainer& detIds() const;
virtual const TrackerGeomDet* idToDetUnit(DetId) const;
virtual const TrackerGeomDet* idToDet(DetId) const;
const DetTypeContainer& detTypes() const {return theDetTypes;}
const DetUnitContainer& detUnits() const {return theDetUnits;}
const DetContainer& dets() const {return theDets;}
const DetIdContainer& detUnitIds() const {return theDetUnitIds;}
const DetIdContainer& detIds() const { return theDetIds;}
const TrackerGeomDet* idToDetUnit(DetId) const;
const TrackerGeomDet* idToDet(DetId) const;

const GeomDetEnumerators::SubDetector geomDetSubDetector(int subdet) const;
unsigned int numberOfLayers(int subdet) const;
Expand Down
31 changes: 2 additions & 29 deletions Geometry/TrackerGeometryBuilder/src/TrackerGeometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ void TrackerGeometry::addDetUnitId(DetId p){
}

void TrackerGeometry::addDet(GeomDet const * p) {
// set index
const_cast<GeomDet *>(p)->setGdetIndex(theDets.size());
Copy link
Contributor

Choose a reason for hiding this comment

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

static analyzer is not happy with (yet another) const_cast in this file.
Why are they even here, should the method signature be changed instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would require modifications in the whole TrackerGeometry building code...
let's wait it stabilize for PhaseII, then we can clean it up.

theDets.push_back(p); // add to vector
theMap.insert(std::make_pair(p->geographicalId().rawId(),p));
DetId id(p->geographicalId());
Expand Down Expand Up @@ -162,17 +164,6 @@ void TrackerGeometry::addDetId(DetId p){
theDetIds.push_back(p);
}

const TrackerGeometry::DetUnitContainer&
TrackerGeometry::detUnits() const
{
return theDetUnits;
}

const TrackerGeometry::DetContainer&
TrackerGeometry::dets() const
{
return theDets;
}

const TrackerGeometry::DetContainer&
TrackerGeometry::detsPXB() const
Expand Down Expand Up @@ -256,24 +247,6 @@ TrackerGeometry::isThere(GeomDetEnumerators::SubDetector subdet) const {
return false;
}

const TrackerGeometry::DetTypeContainer&
TrackerGeometry::detTypes() const
{
return theDetTypes;
}


const TrackerGeometry::DetIdContainer&
TrackerGeometry::detUnitIds() const
{
return theDetUnitIds;
}

const TrackerGeometry::DetIdContainer&
TrackerGeometry::detIds() const
{
return theDetIds;
}
void TrackerGeometry::fillTestMap(const GeometricDet* gd) {

std::string temp = gd->name();
Expand Down
1 change: 1 addition & 0 deletions RecoEgamma/EgammaElectronAlgos/BuildFile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<flags CXXFLAGS="-Ofast"/>
<use name="TrackingTools/MaterialEffects"/>
<use name="FWCore/Framework"/>
<use name="FWCore/ParameterSet"/>
Expand Down
18 changes: 9 additions & 9 deletions RecoEgamma/EgammaElectronAlgos/interface/ElectronUtilities.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ class ExceptionSafeStlPtrCol : public StlColType
template <typename RealType>
RealType normalized_phi( RealType phi )
{
if (phi>CLHEP::pi)
{ phi -= (2*CLHEP::pi) ; }
if (phi<-CLHEP::pi)
{ phi += (2*CLHEP::pi) ; }
constexpr RealType pi(M_PI);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fwyzard lke here.

constexpr RealType pi2(2*M_PI);
if (phi>pi) { phi -= pi2 ; }
if (phi<-pi) { phi += pi2; }
return phi ;
}

Expand Down Expand Up @@ -66,7 +66,7 @@ class EleRelPoint
EleRelPoint( const GlobalPoint & p, const GlobalPoint & origin ) : relP_(p.x()-origin.x(),p.y()-origin.y(),p.z()-origin.z()) {}
double eta() { return relP_.eta() ; }
double phi() { return normalized_phi(relP_.phi()) ; }
double perp() { return sqrt(relP_.x()*relP_.x()+relP_.y()*relP_.y()) ; }
double perp() { return std::sqrt(relP_.x()*relP_.x()+relP_.y()*relP_.y()) ; }
private :
math::XYZVector relP_ ;
} ;
Expand All @@ -82,10 +82,10 @@ class EleRelPointPair
EleRelPointPair( const math::XYZPoint & p1, const GlobalPoint & p2, const GlobalPoint & origin ) : relP1_(p1.x()-origin.x(),p1.y()-origin.y(),p1.z()-origin.z()), relP2_(p2.x()-origin.x(),p2.y()-origin.y(),p2.z()-origin.z()) {}
EleRelPointPair( const GlobalPoint & p1, const math::XYZPoint & p2, const GlobalPoint & origin ) : relP1_(p1.x()-origin.x(),p1.y()-origin.y(),p1.z()-origin.z()), relP2_(p2.x()-origin.x(),p2.y()-origin.y(),p2.z()-origin.z()) {}
EleRelPointPair( const GlobalPoint & p1, const GlobalPoint & p2, const GlobalPoint & origin ) : relP1_(p1.x()-origin.x(),p1.y()-origin.y(),p1.z()-origin.z()), relP2_(p2.x()-origin.x(),p2.y()-origin.y(),p2.z()-origin.z()) {}
double dEta() { return (relP1_.eta()-relP2_.eta()) ; }
double dPhi() { return normalized_phi(relP1_.phi()-relP2_.phi()) ; }
double dZ() { return (relP1_.z()-relP2_.z()) ; }
double dPerp() { return normalized_phi(relP1_.perp()-relP2_.perp()) ; }
auto dEta() { return (relP1_.eta()-relP2_.eta()) ; }
auto dPhi() { return normalized_phi(relP1_.barePhi()-relP2_.barePhi()) ; }
auto dZ() { return (relP1_.z()-relP2_.z()) ; }
auto dPerp() { return (relP1_.perp()-relP2_.perp()) ; }
private :
GlobalVector relP1_ ;
GlobalVector relP2_ ;
Expand Down
28 changes: 11 additions & 17 deletions RecoEgamma/EgammaElectronAlgos/interface/PixelHitMatcher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,28 +50,22 @@ class MagneticField;
class GeometricSearchTracker;
class TrackerGeometry;
class TrackerTopology;
class NavigationSchool;

namespace std{
template<>
struct hash<std::pair<const GeomDet*,GlobalPoint> > {
std::size_t operator()(const std::pair<const GeomDet*,GlobalPoint>& g) const {
std::size_t hsh = 5381;
hsh = ((hsh << 5) + hsh) + (unsigned long)g.first;
hsh = ((hsh << 5) + hsh) + 10000*g.second.x();
hsh = ((hsh << 5) + hsh) + 10000*g.second.y();
hsh = ((hsh << 5) + hsh) + 10000*g.second.z();
return hsh;
auto h1 = std::hash<unsigned long long>()((unsigned long long)g.first);
unsigned long long k; memcpy(&k, &g.second,sizeof(k));
auto h2 = std::hash<unsigned long long>()(k);
return h1 ^ (h2 << 1);
}
};
template<>
struct equal_to<std::pair<const GeomDet*,GlobalPoint> > : public std::binary_function<std::pair<const GeomDet*,GlobalPoint>,std::pair<const GeomDet*,GlobalPoint>,bool> {
bool operator()(const std::pair<const GeomDet*,GlobalPoint>& a,
const std::pair<const GeomDet*,GlobalPoint>& b) const {
return ( a.first == b.first &&
a.second.x() == b.second.x() &&
a.second.y() == b.second.y() &&
a.second.z() == b.second.z() );
return (a.first == b.first) & (a.second == b.second);
}
};
}
Expand Down Expand Up @@ -189,17 +183,18 @@ class PixelHitMatcher

void setEvent( const MeasurementTrackerEvent & event ) ;

std::vector<SeedWithInfo>
compatibleSeeds
( TrajectorySeedCollection * seeds, const GlobalPoint & xmeas,
const GlobalPoint & vprim, float energy, float charge ) ;


std::vector<std::pair<RecHitWithDist,ConstRecHitPointer> >
compatibleHits(const GlobalPoint& xmeas, const GlobalPoint& vprim,
float energy, float charge,
const TrackerTopology *tTopo,
const NavigationSchool& navigationSchool) ;

// compatibleSeeds(edm::Handle<TrajectorySeedCollection> &seeds, const GlobalPoint& xmeas,
std::vector<SeedWithInfo>
compatibleSeeds
( TrajectorySeedCollection * seeds, const GlobalPoint & xmeas,
const GlobalPoint & vprim, float energy, float charge ) ;

std::vector<CLHEP::Hep3Vector> predicted1Hits() ;
std::vector<CLHEP::Hep3Vector> predicted2Hits();
Expand Down Expand Up @@ -236,7 +231,6 @@ class PixelHitMatcher

bool searchInTIDTEC_ ;
bool useRecoVertex_ ;
std::unordered_map<const GeomDet*, TrajectoryStateOnSurface> mapTsos_fast_;
std::unordered_map<std::pair<const GeomDet*,GlobalPoint>, TrajectoryStateOnSurface> mapTsos2_fast_;
std::vector<GlobalPoint> hit_gp_map_;
} ;
Expand Down
Loading