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

Speed seeding for HLT #13819

Merged
merged 13 commits into from Apr 4, 2016
27 changes: 17 additions & 10 deletions DataFormats/GeometrySurface/src/BoundSpan.cc
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
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
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
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
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
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
@@ -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
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
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