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

Added "/dd4hep::cm" to the CSCGeometryParsFromDD Class #32526

Merged
merged 6 commits into from Dec 25, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
104 changes: 73 additions & 31 deletions Geometry/CSCGeometryBuilder/src/CSCGeometryParsFromDD.cc
Expand Up @@ -21,7 +21,6 @@
#include "Geometry/CSCGeometry/src/CSCWireGroupPackage.h"
#include "CondFormats/GeometryObjects/interface/CSCRecoDigiParameters.h"
#include "CondFormats/GeometryObjects/interface/RecoIdealGeometry.h"
#include "CLHEP/Units/GlobalSystemOfUnits.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "DetectorDescription/DDCMS/interface/DDFilteredView.h"
#include "DetectorDescription/DDCMS/interface/DDCompactView.h"
Expand All @@ -31,11 +30,14 @@

using namespace std;
using namespace cms_units::operators;
using namespace geant_units::operators;

CSCGeometryParsFromDD::CSCGeometryParsFromDD() : myName("CSCGeometryParsFromDD") {}

CSCGeometryParsFromDD::~CSCGeometryParsFromDD() {}

//ddd

bool CSCGeometryParsFromDD::build(const DDCompactView* cview,
const MuonGeometryConstants& muonConstants,
RecoIdealGeometry& rig,
Expand Down Expand Up @@ -65,6 +67,9 @@ bool CSCGeometryParsFromDD::build(const DDCompactView* cview,
std::vector<double> gtran(3);
std::vector<double> grmat(9);
std::vector<double> trm(9);

edm::LogVerbatim("CSCGeometryParsFromDD") << "(0) CSCGeometryParsFromDD - DDD ";

while (doSubDets) {
spec = fv.specifics();
spit = spec.begin();
Expand All @@ -91,6 +96,10 @@ bool CSCGeometryParsFromDD::build(const DDCompactView* cview,
int jchamber = detid.chamber();
int jlayer = detid.layer();

edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(1) detId: " << id << " jendcap: " << jendcap << " jstation: " << jstation << " jring: " << jring
<< " jchamber: " << jchamber << " jlayer: " << jlayer;

// Package up the wire group info as it's decoded
CSCWireGroupPackage wg;
uparvals.clear();
Expand Down Expand Up @@ -137,16 +146,21 @@ bool CSCGeometryParsFromDD::build(const DDCompactView* cview,
}
} else if (it->second.name() == "WireSpacing") {
wg.wireSpacing = it->second.doubles()[0];
edm::LogVerbatim("CSCGeometryParsFromDD") << "(2) wireSpacing: " << wg.wireSpacing;
} else if (it->second.name() == "AlignmentPinToFirstWire") {
wg.alignmentPinToFirstWire = it->second.doubles()[0];
edm::LogVerbatim("CSCGeometryParsFromDD") << "(3) alignmentPinToFirstWire: " << wg.alignmentPinToFirstWire;
} else if (it->second.name() == "TotNumWireGroups") {
wg.numberOfGroups = int(it->second.doubles()[0]);
} else if (it->second.name() == "LengthOfFirstWire") {
wg.narrowWidthOfWirePlane = it->second.doubles()[0];
edm::LogVerbatim("CSCGeometryParsFromDD") << "(4) narrowWidthOfWirePlane: " << wg.narrowWidthOfWirePlane;
} else if (it->second.name() == "LengthOfLastWire") {
wg.wideWidthOfWirePlane = it->second.doubles()[0];
edm::LogVerbatim("CSCGeometryParsFromDD") << "(5) wideWidthOfWirePlane: " << wg.wideWidthOfWirePlane;
} else if (it->second.name() == "RadialExtentOfWirePlane") {
wg.lengthOfWirePlane = it->second.doubles()[0];
edm::LogVerbatim("CSCGeometryParsFromDD") << "(6) lengthOfWirePlane: " << wg.lengthOfWirePlane;
}
}
}
Expand Down Expand Up @@ -198,22 +212,28 @@ bool CSCGeometryParsFromDD::build(const DDCompactView* cview,

LogTrace(myName) << myName << ": noOfAnonParams=" << noOfAnonParams;
LogTrace(myName) << myName << ": fill fpar...";
LogTrace(myName) << myName << ": dpars are... " << dpar[4] / cm << ", " << dpar[8] / cm << ", " << dpar[3] / cm
<< ", " << dpar[0] / cm;
LogTrace(myName) << myName << ": dpars are... " << convertMmToCm(dpar[4]) << ", " << convertMmToCm(dpar[8]) << ", "
<< convertMmToCm(dpar[3]) << ", " << convertMmToCm(dpar[0]);
edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(7) dpar[4]: " << convertMmToCm(dpar[4]) << " dpar[8]: " << convertMmToCm(dpar[8])
<< " dpar[3]: " << convertMmToCm(dpar[3]) << " dpar[0]: " << convertMmToCm(dpar[0]);

fpar.emplace_back((dpar[4] / cm));
fpar.emplace_back((dpar[8] / cm));
fpar.emplace_back((dpar[3] / cm));
fpar.emplace_back((dpar[0] / cm));
fpar.emplace_back(convertMmToCm(dpar[4]));
fpar.emplace_back(convertMmToCm(dpar[8]));
fpar.emplace_back(convertMmToCm(dpar[3]));
fpar.emplace_back(convertMmToCm(dpar[0]));

LogTrace(myName) << myName << ": fill gtran...";

gtran[0] = (float)1.0 * (fv.translation().X() / cm);
gtran[1] = (float)1.0 * (fv.translation().Y() / cm);
gtran[2] = (float)1.0 * (fv.translation().Z() / cm);
gtran[0] = (float)1.0 * (convertMmToCm(fv.translation().X()));
gtran[1] = (float)1.0 * (convertMmToCm(fv.translation().Y()));
gtran[2] = (float)1.0 * (convertMmToCm(fv.translation().Z()));

LogTrace(myName) << myName << ": gtran[0]=" << gtran[0] << ", gtran[1]=" << gtran[1] << ", gtran[2]=" << gtran[2];

edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(8) gtran[0]: " << gtran[0] << " gtran[1]: " << gtran[1] << " gtran[2]: " << gtran[2];

LogTrace(myName) << myName << ": fill grmat...";

fv.rotation().GetComponents(trm.begin(), trm.end());
Expand Down Expand Up @@ -369,6 +389,9 @@ bool CSCGeometryParsFromDD::build(const cms::DDCompactView* cview,
std::vector<double> gtran(3);
std::vector<double> grmat(9);
std::vector<double> trm(9);

edm::LogVerbatim("CSCGeometryParsFromDD") << "(0) CSCGeometryParsFromDD - DD4HEP ";

while (fv.firstChild()) {
MuonGeometryNumbering mbn(muonConstants);
CSCNumberingScheme cscnum(muonConstants);
Expand All @@ -381,6 +404,10 @@ bool CSCGeometryParsFromDD::build(const cms::DDCompactView* cview,
int jchamber = detid.chamber();
int jlayer = detid.layer();

edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(1) detId: " << id << " jendcap: " << jendcap << " jstation: " << jstation << " jring: " << jring
<< " jchamber: " << jchamber << " jlayer: " << jlayer;

// Package up the wire group info as it's decoded
CSCWireGroupPackage wg;
uparvals.clear();
Expand Down Expand Up @@ -426,24 +453,31 @@ bool CSCGeometryParsFromDD::build(const cms::DDCompactView* cview,
}

auto wirespacing = fv.get<double>("WireSpacing");
wg.wireSpacing = static_cast<double>(wirespacing);
wg.wireSpacing = static_cast<double>(wirespacing / dd4hep::mm);
edm::LogVerbatim("CSCGeometryParsFromDD") << "(2) wireSpacing: " << wg.wireSpacing / dd4hep::mm;
Copy link
Contributor

@cvuosalo cvuosalo Dec 22, 2020

Choose a reason for hiding this comment

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

These debug print-out statements are not correct. Line 456 correctly divides wirespacing by 0.1 to put wg.wireSpacing into mm.

wg.wireSpacing = static_cast<double>(wirespacing / dd4hep::mm);

Then

edm::LogVerbatim("CSCGeometryParsFromDD") << "(2) wireSpacing: " << wg.wireSpacing / dd4hep::mm;

divides wg.wireSpacing by 0.1 again, which is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cvuosalo : ok, I'll fix them


auto alignmentpintofirstwire = fv.get<double>("AlignmentPinToFirstWire");
wg.alignmentPinToFirstWire = static_cast<double>(alignmentpintofirstwire);
wg.alignmentPinToFirstWire = static_cast<double>(alignmentpintofirstwire / dd4hep::mm);
edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(3) alignmentPinToFirstWire: " << wg.alignmentPinToFirstWire / dd4hep::mm;

auto totnumwiregroups = fv.get<double>("TotNumWireGroups");
wg.numberOfGroups = static_cast<int>(totnumwiregroups);

auto lengthoffirstwire = fv.get<double>("LengthOfFirstWire");
wg.narrowWidthOfWirePlane = static_cast<double>(lengthoffirstwire);
wg.narrowWidthOfWirePlane = static_cast<double>(lengthoffirstwire / dd4hep::mm);
edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(4) narrowWidthOfWirePlane: " << wg.narrowWidthOfWirePlane / dd4hep::mm;

auto lengthoflastwire = fv.get<double>("LengthOfLastWire");
wg.wideWidthOfWirePlane = static_cast<double>(lengthoflastwire);
wg.wideWidthOfWirePlane = static_cast<double>(lengthoflastwire / dd4hep::mm);
edm::LogVerbatim("CSCGeometryParsFromDD") << "(5) wideWidthOfWirePlane: " << wg.wideWidthOfWirePlane / dd4hep::mm;

auto radialextentofwireplane = fv.get<double>("RadialExtentOfWirePlane");
wg.lengthOfWirePlane = static_cast<double>(radialextentofwireplane);
wg.lengthOfWirePlane = static_cast<double>(radialextentofwireplane / dd4hep::mm);
edm::LogVerbatim("CSCGeometryParsFromDD") << "(6) lengthOfWirePlane: " << wg.lengthOfWirePlane / dd4hep::mm;

uparvals.emplace_back((wg.wireSpacing) * 10.0);
uparvals.emplace_back(wg.wireSpacing);
uparvals.emplace_back(wg.alignmentPinToFirstWire);
uparvals.emplace_back(wg.numberOfGroups);
uparvals.emplace_back(wg.narrowWidthOfWirePlane);
Expand Down Expand Up @@ -477,25 +511,36 @@ bool CSCGeometryParsFromDD::build(const cms::DDCompactView* cview,
std::transform(
dpar.begin(), dpar.end(), dpar.begin(), [](double i) -> double { return cms_rounding::roundIfNear0(i); });

fpar.emplace_back((dpar[1]));
fpar.emplace_back((dpar[2]));
fpar.emplace_back((dpar[3]));
fpar.emplace_back((dpar[4]));
fpar.emplace_back((dpar[1] / dd4hep::cm));
fpar.emplace_back((dpar[2] / dd4hep::cm));
fpar.emplace_back((dpar[3] / dd4hep::cm));
fpar.emplace_back((dpar[4] / dd4hep::cm));
edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(7) - Subtraction - dpar[1] (ddd dpar[4]): " << dpar[1] / dd4hep::cm
<< " (ddd dpar[8]): " << dpar[2] / dd4hep::cm << " dpar[3] (as ddd): " << dpar[3] / dd4hep::cm
<< " dpar[4] (ddd dpar[0]): " << dpar[4] / dd4hep::cm;
} else {
dpar = fv.parameters();

std::transform(
dpar.begin(), dpar.end(), dpar.begin(), [](double i) -> double { return cms_rounding::roundIfNear0(i); });

fpar.emplace_back((dpar[0]));
fpar.emplace_back((dpar[1]));
fpar.emplace_back((dpar[2]));
fpar.emplace_back((dpar[3]));
fpar.emplace_back((dpar[0] / dd4hep::cm));
fpar.emplace_back((dpar[1] / dd4hep::cm));
fpar.emplace_back((dpar[2] / dd4hep::cm));
fpar.emplace_back((dpar[3] / dd4hep::cm));
edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(7)Bis - Else - dpar[0]: " << dpar[0] / dd4hep::cm << " dpar[1]: " << dpar[1] / dd4hep::cm
<< " dpar[2]: " << dpar[2] / dd4hep::cm << " dpar[3]: " << dpar[3] / dd4hep::cm;
}

gtran[0] = static_cast<float>(fv.translation().X());
gtran[1] = static_cast<float>(fv.translation().Y());
gtran[2] = static_cast<float>(fv.translation().Z());
gtran[0] = static_cast<float>(fv.translation().X() / dd4hep::cm);
gtran[1] = static_cast<float>(fv.translation().Y() / dd4hep::cm);
gtran[2] = static_cast<float>(fv.translation().Z() / dd4hep::cm);

edm::LogVerbatim("CSCGeometryParsFromDD")
<< "(8) gtran[0]: " << gtran[0] / dd4hep::cm << " gtran[1]: " << gtran[1] / dd4hep::cm
<< " gtran[2]: " << gtran[2] / dd4hep::cm;
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed lines 557-562 contain a useless loop. Those lines can be changed to:

 if (wg.numberOfGroups == 0) {
       LogTrace(myName) << myName << " wg.numberOfGroups == 0 ";
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cvuosalo : done


std::transform(
gtran.begin(), gtran.end(), gtran.begin(), [](double i) -> double { return cms_rounding::roundIfNear0(i); });
Expand All @@ -510,10 +555,7 @@ bool CSCGeometryParsFromDD::build(const cms::DDCompactView* cview,
}
}

if (wg.numberOfGroups != 0) {
for (size_t i = 0; i < wg.consecutiveGroups.size(); i++) {
}
} else {
if (wg.numberOfGroups == 0) {
LogTrace(myName) << myName << " wg.numberOfGroups == 0 ";
}

Expand Down
Expand Up @@ -9,12 +9,18 @@
input = cms.untracked.int32(1)
)


process.load('Configuration.StandardSequences.DD4hep_GeometrySim_cff')
process.load("FWCore.MessageLogger.MessageLogger_cfi")
process.load("Geometry.MuonNumbering.muonGeometryConstants_cff")
process.load("Geometry.CSCGeometryBuilder.cscGeometry_cfi")

process.MessageLogger = cms.Service("MessageLogger",
destinations = cms.untracked.vstring('myLog'),
myLog = cms.untracked.PSet(
threshold = cms.untracked.string('INFO'),
)
)

process.CSCGeometryESModule.applyAlignment = False

#
Expand All @@ -25,7 +31,7 @@
#
process.valid = cms.EDAnalyzer("CSCGeometryValidate",
infileName = cms.untracked.string('cmsRecoGeom-2021.root'),
outfileName = cms.untracked.string('validateCSCGeometry.root'),
outfileName = cms.untracked.string('validateCSCGeometryDD4HEP.root'),
tolerance = cms.untracked.int32(7)
)

Expand Down
Expand Up @@ -14,6 +14,13 @@
process.load("Geometry.MuonNumbering.muonNumberingInitialization_cfi")
process.load("Geometry.CSCGeometryBuilder.cscGeometry_cfi")

process.MessageLogger = cms.Service("MessageLogger",
destinations = cms.untracked.vstring('myLog'),
myLog = cms.untracked.PSet(
threshold = cms.untracked.string('INFO'),
)
)

process.CSCGeometryESModule.applyAlignment = False

#
Expand All @@ -24,7 +31,7 @@
#
process.valid = cms.EDAnalyzer("CSCGeometryValidate",
infileName = cms.untracked.string('cmsRecoGeom-2021.root'),
outfileName = cms.untracked.string('validateCSCGeometry2.root'),
outfileName = cms.untracked.string('validateCSCGeometryDDD.root'),
tolerance = cms.untracked.int32(7)
)

Expand Down