Skip to content

Run3-gex157A Replace use of cout's with LogVerbatim in Geometry/CaloTopology and Geometry/CommonTopologies #41173

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

Merged
cmsbuild merged 9 commits into from
Mar 29, 2023
Merged

Run3-gex157A Replace use of cout's with LogVerbatim in Geometry/CaloTopology and Geometry/CommonTopologies #41173

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
02ee2a2
Replace use of cout's with LogVerbatim in Geometry/CaloTopology and G…
Mar 24, 2023
f224b45
Code check
Mar 24, 2023
47f67fc
Restore some fixes other than those on cout's
Mar 27, 2023
c3b1e59
Add on the essagelogger request
Mar 28, 2023
d2a20d3
Try to overcome unit test error
Mar 28, 2023
8de4ec7
Code check
Mar 28, 2023
2d37d74
Restore runTest
Mar 28, 2023
d3c2821
reuse cout to avoid unit test issue
Mar 29, 2023
d70fd54
Code check
Mar 29, 2023
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
5 changes: 3 additions & 2 deletions Geometry/CaloTopology/interface/EcalBarrelHardcodedTopology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <vector>
#include <iostream>
#include "DataFormats/EcalDetId/interface/EBDetId.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "Geometry/CaloTopology/interface/CaloSubdetectorTopology.h"

class EcalBarrelHardcodedTopology final : public CaloSubdetectorTopology {
Expand Down Expand Up @@ -54,13 +55,13 @@ class EcalBarrelHardcodedTopology final : public CaloSubdetectorTopology {
}

std::vector<DetId> up(const DetId& /*id*/) const override {
std::cout << "EcalBarrelHardcodedTopology::up() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalBarrelHardcodedTopology::up() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}

std::vector<DetId> down(const DetId& /*id*/) const override {
std::cout << "EcalBarrelHardcodedTopology::down() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalBarrelHardcodedTopology::down() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}
Expand Down
5 changes: 3 additions & 2 deletions Geometry/CaloTopology/interface/EcalBarrelTopology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <vector>
#include <iostream>
#include "DataFormats/EcalDetId/interface/EBDetId.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "Geometry/CaloTopology/interface/CaloSubdetectorTopology.h"
#include "Geometry/CaloGeometry/interface/CaloGeometry.h"
#include "Geometry/CaloGeometry/interface/CaloSubdetectorGeometry.h"
Expand Down Expand Up @@ -63,13 +64,13 @@ class EcalBarrelTopology final : public CaloSubdetectorTopology {
}

std::vector<DetId> up(const DetId& /*id*/) const override {
std::cout << "EcalBarrelTopology::up() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalBarrelTopology::up() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}

std::vector<DetId> down(const DetId& /*id*/) const override {
std::cout << "EcalBarrelTopology::down() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalBarrelTopology::down() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}
Expand Down
5 changes: 3 additions & 2 deletions Geometry/CaloTopology/interface/EcalEndcapHardcodedTopology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <vector>
#include <iostream>
#include "DataFormats/EcalDetId/interface/EEDetId.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "Geometry/CaloTopology/interface/CaloSubdetectorTopology.h"

class EcalEndcapHardcodedTopology final : public CaloSubdetectorTopology {
Expand Down Expand Up @@ -54,13 +55,13 @@ class EcalEndcapHardcodedTopology final : public CaloSubdetectorTopology {
}

std::vector<DetId> up(const DetId& /*id*/) const override {
std::cout << "EcalEndcapHardcodedTopology::up() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalEndcapHardcodedTopology::up() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}

std::vector<DetId> down(const DetId& /*id*/) const override {
std::cout << "EcalEndcapHardcodedTopology::down() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalEndcapHardcodedTopology::down() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}
Expand Down
5 changes: 3 additions & 2 deletions Geometry/CaloTopology/interface/EcalEndcapTopology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define GEOMETRY_CALOTOPOLOGY_ECALENDCAPTOPOLOGY_H 1

#include "DataFormats/EcalDetId/interface/EEDetId.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "Geometry/CaloTopology/interface/CaloSubdetectorTopology.h"
#include "Geometry/CaloGeometry/interface/CaloGeometry.h"
#include "Geometry/CaloGeometry/interface/CaloSubdetectorGeometry.h"
Expand Down Expand Up @@ -63,13 +64,13 @@ class EcalEndcapTopology final : public CaloSubdetectorTopology {
}

std::vector<DetId> up(const DetId& /*id*/) const override {
std::cout << "EcalBarrelTopology::up() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalBarrelTopology::up() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}

std::vector<DetId> down(const DetId& /*id*/) const override {
std::cout << "EcalBarrelTopology::down() not yet implemented" << std::endl;
edm::LogVerbatim("CaloTopology") << "EcalBarrelTopology::down() not yet implemented";
std::vector<DetId> vNeighborsDetId;
return vNeighborsDetId;
}
Expand Down
3 changes: 2 additions & 1 deletion Geometry/CaloTopology/src/EcalTrigTowerConstituentsMap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ EcalTrigTowerDetId EcalTrigTowerConstituentsMap::towerOf(const DetId& id) const
//..........from file and done only for 1 quadrant
//move from quadrant 1 to the actual one:
// phiTower = changeTowerQuadrant(phiTower, 1, originalId.iquadrant());
// std::cout << originalId.zside() << " " << etaTower << " " << phiTower << std::endl;
edm::LogVerbatim("EcalTrigTowerConstituentsMap")
<< "EcalTrigTowerConstituentsMap " << originalId.zside() << " " << etaTower << " " << phiTower;
return EcalTrigTowerDetId(originalId.zside(), EcalEndcap, etaTower, phiTower);
}
}
Expand Down
5 changes: 3 additions & 2 deletions Geometry/CommonTopologies/src/RectangularStripTopology.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "Geometry/CommonTopologies/interface/RectangularStripTopology.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"

#include <iostream>
#include <cmath>
Expand All @@ -9,8 +10,8 @@ RectangularStripTopology::RectangularStripTopology(int ns, float p, float l)
theOffset = -0.5f * theNumberOfStrips * thePitch;

#ifdef VERBOSE
cout << "Constructing RectangularStripTopology with"
<< " nstrips = " << ns << " pitch = " << p << " length = " << l << endl;
edm::LogVerbatim("CommonTopologies") << "Constructing RectangularStripTopology with"
<< " nstrips = " << ns << " pitch = " << p << " length = " << l;
#endif
}

Expand Down
9 changes: 4 additions & 5 deletions Geometry/CommonTopologies/src/TkRadialStripTopology.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ namespace {
struct Stat {
Stat(const char* in) : name(in){};
~Stat() {
std::cout << name << ": atan0 calls tot/large/over1: " << natan << "/" << nlarge << "/" << over1 << std::endl;
edm::LogVerbatim("CommonTopologies")
<< name << ": atan0 calls tot/large/over1: " << natan << "/" << nlarge << "/" << over1;
}

void add(float t) {
Expand Down Expand Up @@ -133,9 +134,7 @@ float TkRadialStripTopology::coveredStrips(const LocalPoint& lp1, const LocalPoi
#ifdef MATH_STS
statS.add(t);
#endif
// std::cout << "atans " << atanClip(t)
// <<" "<< std::atan2(lp1.x(),yDistanceToIntersection(lp1.y()) )
// -std::atan2(lp2.x(),yDistanceToIntersection(lp2.y()) ) << std::endl;
// edm::LogVerbatim("CommonTopologies") << "atans " << atanClip(t) << " " << std::atan2(lp1.x(), yDistanceToIntersection(lp1.y())) - std::atan2(lp2.x(),yDistanceToIntersection(lp2.y()));
// clip???
return atanClip(t) * theAWidthInverse;
// return (measurementPosition(lp1)-measurementPosition(lp2)).x();
Expand Down Expand Up @@ -231,7 +230,7 @@ float TkRadialStripTopology::localPitch(const LocalPoint& lp) const {
float y = yDistanceToIntersection( lp.y() );
float x = std::abs(lp.x());
float myP = y*(y*theTanAW+x)/(y-theTanAW*x)-x; // (y*theTanAW+x)/(1.f-theTanAW*x/y)-x;
std::cout << "localPitch " << p << " " << myP << std::endl;
edm::LogVerbatim("CommonTopologies") << "localPitch " << p << " " << myP;

return p;

Expand Down
17 changes: 10 additions & 7 deletions Geometry/CommonTopologies/src/TrapezoidalStripTopology.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
#include "Geometry/CommonTopologies/interface/TrapezoidalStripTopology.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"

#include <iostream>
#include <cmath>
#include <algorithm>

//#define EDM_ML_DEBUG

TrapezoidalStripTopology::TrapezoidalStripTopology(int ns, float p, float l, float r0)
: theNumberOfStrips(ns), thePitch(p), theDistToBeam(r0), theDetHeight(l) {
theOffset = -theNumberOfStrips / 2. * thePitch;
theYAxOr = 1;
#ifdef VERBOSE
cout << "Constructing TrapezoidalStripTopology with"
<< " nstrips = " << ns << " pitch = " << p << " length = " << l << " r0 =" << r0 << endl;
#ifdef EDM_ML_DEBUG
edm::LogVerbatim("CommonTopologies") << "Constructing TrapezoidalStripTopology with nstrips = " << ns
<< " pitch = " << p << " length = " << l << " r0 =" << r0;
#endif
}

TrapezoidalStripTopology::TrapezoidalStripTopology(int ns, float p, float l, float r0, int yAx)
: theNumberOfStrips(ns), thePitch(p), theDistToBeam(r0), theDetHeight(l), theYAxOr(yAx) {
theOffset = -theNumberOfStrips / 2. * thePitch;
#ifdef VERBOSE
cout << "Constructing TrapezoidalStripTopology with"
<< " nstrips = " << ns << " pitch = " << p << " length = " << l << " r0 =" << r0 << " yAxOrientation =" << yAx
<< endl;
#ifdef EDM_ML_DEBUG
edm::LogVerbatim("CommonTopologies") << "Constructing TrapezoidalStripTopology with nstrips = " << ns
<< " pitch = " << p << " length = " << l << " r0 =" << r0
<< " yAxOrientation =" << yAx;
#endif
}

Expand Down
5 changes: 2 additions & 3 deletions Geometry/CommonTopologies/src/TwoBowedSurfacesDeformation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ SurfaceDeformation::Local2DVector TwoBowedSurfacesDeformation::positionCorrectio

// treatment of different widthes at high/low y could be done by theRelWidthLowY or so
// if (widthLowY > 0. && widthHighY != widthLowY) {
// std::cout << "SurfaceDeformation::positionCorrection2Bowed: Cannot yet deal "
// << " with different widthes, take " << widthHighY << " not " << widthLowY
// << std::endl;
// edm::LogVerbatim("CommonTopologies") << "SurfaceDeformation::positionCorrection2Bowed: Cannot yet deal "
// << " with different widthes, take " << widthHighY << " not " << widthLowY;
// }
// const double width = widthHighY;

Expand Down
35 changes: 18 additions & 17 deletions Geometry/CommonTopologies/test/ValidateRadial.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "Geometry/TrackerGeometryBuilder/interface/StripGeomDetUnit.h"
#include "Geometry/Records/interface/TrackerDigiGeometryRecord.h"
#include "FWCore/Framework/interface/ESHandle.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/Framework/interface/MakerMacros.h"
#include "Geometry/TrackerGeometryBuilder/interface/TrackerGeometry.h"
Expand Down Expand Up @@ -40,7 +41,7 @@ ValidateRadial::ValidateRadial(const edm::ParameterSet& cfg)
printOut_(cfg.getParameter<bool>("PrintOut")),
posOnly_(cfg.getParameter<bool>("PosOnly")),
tokTrackerGeometry_(esConsumes<TrackerGeometry, TrackerDigiGeometryRecord>()) {
std::cout << "I'm ALIVE" << std::endl;
Copy link
Contributor

@perrotta perrotta Mar 28, 2023

Choose a reason for hiding this comment

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

The test script is looking for the string "ALIVE" in the output, see

cmssw/Geometry/CommonTopologies/test/runTest.sh

Line 5 in 47f67fc

cmsRun ${LOCAL_TEST_DIR}/python/ValidateRadial_cfg.py | grep "ALIVE" > /dev/null

If you move to LogVerbatim the script must be updated accordingly

edm::LogVerbatim("CommonTopologies") << "I'm ALIVE";
}

void ValidateRadial::analyze(const edm::Event& e, const edm::EventSetup& es) {
Expand Down Expand Up @@ -68,7 +69,7 @@ std::vector<const TkRadialStripTopology*> ValidateRadial::get_list_of_radial_top
for (unsigned int radial_detid : radial_detids) {
auto g = dynamic_cast<const StripGeomDetUnit*>(theTrackerGeometry->idToDet(radial_detid));
if (!g)
std::cout << "no geom for " << radial_detid << std::endl;
edm::LogVerbatim("CommonTopologies") << "no geom for " << radial_detid;
auto const topol = &g->specificTopology();
const TkRadialStripTopology* rt = nullptr;
auto const proxyT = dynamic_cast<const ProxyStripTopology*>(topol);
Expand All @@ -77,7 +78,7 @@ std::vector<const TkRadialStripTopology*> ValidateRadial::get_list_of_radial_top
else
rt = dynamic_cast<const TkRadialStripTopology*>(topol);
if (!rt)
std::cout << "no radial topology for " << radial_detid << std::endl;
edm::LogVerbatim("CommonTopologies") << "no radial topology for " << radial_detid;
else
topos.emplace_back(rt);
}
Expand Down Expand Up @@ -105,22 +106,22 @@ void compare(const TkRadialStripTopology& t,
const float onewstrip = ot.strip(lp);

if (fabs(lp.x() - olp.x()) > 0.001)
std::cout << "FAILED " << lp.x() << " " << olp.x() << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << lp.x() << " " << olp.x();
if (fabs(lp.y() - olp.y()) > 0.001)
std::cout << "FAILED " << lp.y() << " " << olp.y() << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << lp.y() << " " << olp.y();
if (fabs(newstrip - onewstrip) > 0.001)
std::cout << "FAILED " << newstrip << " " << onewstrip << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << newstrip << " " << onewstrip;
if (fabs(le.xx() - ole.xx()) > 0.001)
std::cout << "FAILED " << le.xx() << " " << ole.xx() << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << le.xx() << " " << ole.xx();
if (fabs(le.yy() - ole.yy()) > 0.001)
std::cout << "FAILED " << le.yy() << " " << ole.xy() << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << le.yy() << " " << ole.xy();
if (fabs(le.xy() - ole.xy()) > 0.001)
std::cout << "FAILED " << le.xy() << " " << ole.yy() << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << le.xy() << " " << ole.yy();

if (fabs(mp.x() - omp.x()) > 0.001)
std::cout << "FAILED " << mp.x() << " " << omp.x() << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << mp.x() << " " << omp.x();
if (fabs(me.uu() - ome.uu()) > 0.001)
std::cout << "FAILED " << me.uu() << " " << ome.uu() << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED " << me.uu() << " " << ome.uu();
}

void ValidateRadial::test_topology(const TkRadialStripTopology* t, unsigned i) {
Expand All @@ -131,8 +132,8 @@ void ValidateRadial::test_topology(const TkRadialStripTopology* t, unsigned i) {
t->yAxisOrientation(),
t->yCentreOfStripPlane());

std::cout << "\nTK\n" << *t << std::endl;
std::cout << "\nCSC\n" << oldt << std::endl;
edm::LogVerbatim("CommonTopologies") << "\nTK\n" << *t;
edm::LogVerbatim("CommonTopologies") << "\nCSC\n" << oldt;

TProfile prof(("se2limit1" + std::to_string(i)).c_str(),
"Precision Limit of recoverable strip error (1st order);strip;strip error",
Expand Down Expand Up @@ -163,7 +164,7 @@ void ValidateRadial::test_topology(const TkRadialStripTopology* t, unsigned i) {
}

ValidateRadial::~ValidateRadial() {
std::cout << "ValidateRadial max UU, max UV " << maxerrU << " " << maxerrUV << std::endl;
edm::LogVerbatim("CommonTopologies") << "ValidateRadial max UU, max UV " << maxerrU << " " << maxerrUV;
}

bool ValidateRadial::pass_frame_change_test(const TkRadialStripTopology* t,
Expand Down Expand Up @@ -194,9 +195,9 @@ bool ValidateRadial::pass_frame_change_test(const TkRadialStripTopology* t,
const bool passe = (secondOrder ? pass2e : pass1e);

if (printOut_ && ((!passp) || ((!posOnly_) && (!passe))))
std::cout << "FAILED "
<< "(" << strip << ", " << newstrip << ", " << mp.x() << ")\t"
<< "(" << stripErr2 << ", " << me.uu() << ")\t\t" << (me.uv()) << std::endl;
edm::LogVerbatim("CommonTopologies") << "FAILED "
<< "(" << strip << ", " << newstrip << ", " << mp.x() << ")\t"
<< "(" << stripErr2 << ", " << me.uu() << ")\t\t" << (me.uv());
return passp & passe;
}

Expand Down
27 changes: 14 additions & 13 deletions Geometry/CommonTopologies/test/testDetSort.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "Geometry/CommonTopologies/interface/DetSorting.h"
#include "DataFormats/GeometrySurface/interface/BoundPlane.h"
#include "DataFormats/GeometrySurface/interface/RectangularPlaneBounds.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"

#include <iostream>
#include <iterator>
Expand All @@ -29,8 +30,8 @@ class MyDet : public GeomDet {
// A simple helper for printing the object
struct dump {
void operator()(const GeomDet& o) {
cout << o.position() << " R : " << o.position().perp() << " Phi: " << o.position().phi()
<< " Z : " << o.position().z() << endl;
edm::LogVerbatim("CommonTopologies") << o.position() << " R : " << o.position().perp()
<< " Phi: " << o.position().phi() << " Z : " << o.position().z();
}
void operator()(const GeomDet* o) { operator()(*o); }
};
Expand All @@ -41,51 +42,51 @@ int main() {
//

// Fill the vector to be sorted
vector<MyDet> v;
std::vector<MyDet> v;
v.emplace_back(MyDet(PositionType(2, 1, 1)));
v.emplace_back(MyDet(PositionType(1, 1, 2)));
v.emplace_back(MyDet(PositionType(1, 2, 3)));
v.emplace_back(MyDet(PositionType(2, 2, 4)));

cout << "Original vector: " << endl;
edm::LogVerbatim("CommonTopologies") << "Original vector: ";
for_each(v.begin(), v.end(), dump());

cout << "Sort in R : " << endl;
edm::LogVerbatim("CommonTopologies") << "Sort in R : ";
// Here we sort in R
precomputed_value_sort(v.begin(), v.end(), DetR());
for_each(v.begin(), v.end(), dump());

cout << "Sort in phi : " << endl;
edm::LogVerbatim("CommonTopologies") << "Sort in phi : ";
// Here we sort in phi
precomputed_value_sort(v.begin(), v.end(), DetPhi());
for_each(v.begin(), v.end(), dump());

cout << "Sort in z : " << endl;
edm::LogVerbatim("CommonTopologies") << "Sort in z : ";
// Here we sort in Z
precomputed_value_sort(v.begin(), v.end(), DetZ());
for_each(v.begin(), v.end(), dump());

//
// Now do the same with a vector of pointers
//
cout << endl << "Again with pointers" << endl;
edm::LogVerbatim("CommonTopologies") << std::endl << "Again with pointers";

vector<const MyDet*> vp;
for (vector<MyDet>::const_iterator i = v.begin(); i != v.end(); i++) {
std::vector<const MyDet*> vp;
for (std::vector<MyDet>::const_iterator i = v.begin(); i != v.end(); i++) {
vp.emplace_back(&(*i));
}

cout << "Sort in R : " << endl;
edm::LogVerbatim("CommonTopologies") << "Sort in R : ";
// Here we sort in R
precomputed_value_sort(vp.begin(), vp.end(), DetR());
for_each(vp.begin(), vp.end(), dump());

cout << "Sort in phi : " << endl;
edm::LogVerbatim("CommonTopologies") << "Sort in phi : ";
// Here we sort in phi
precomputed_value_sort(vp.begin(), vp.end(), DetPhi());
for_each(vp.begin(), vp.end(), dump());

cout << "Sort in z : " << endl;
edm::LogVerbatim("CommonTopologies") << "Sort in z : ";
// Here we sort in Z
precomputed_value_sort(vp.begin(), vp.end(), DetZ());
for_each(vp.begin(), vp.end(), dump());
Expand Down