Skip to content

Make units dd4hep-dependent in PPS #32538

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 4 commits into from
Jan 6, 2021
Merged

Make units dd4hep-dependent in PPS #32538

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c63883d
Make units of values from XMLs dd4hep-dependent. This will allow the …
ghugo83 Dec 18, 2020
babaaf9
Update comments regarding future change of dd4hep unit
ghugo83 Dec 21, 2020
dcbe32f
Address comments
ghugo83 Jan 4, 2021
4c4d7c4
scram b code-format
ghugo83 Jan 4, 2021
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
6 changes: 3 additions & 3 deletions Geometry/VeryForwardGeometryBuilder/interface/DetGeomDesc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class CTPPSRPAlignmentCorrectionData;
\endverbatim
*
* July 2020: Migrated to DD4hep
* To avoid any regression with values from XMLs / Geant4, all lengths are converted from cm (DD4hep) to mm.
* To avoid any regression with values from XMLs / Geant4, all lengths are converted from DD4hep unit to mm.
*
**/

Expand Down Expand Up @@ -80,7 +80,7 @@ class DetGeomDesc {
// params() is left for general access to solid shape parameters, but should be used
// only with great care, for two reasons: 1. order of parameters may possibly change from
// a version to another of DD4hep; 2. length parameters unit is cm while PPS uses mm.
Copy link
Contributor

@cvuosalo cvuosalo Dec 21, 2020

Choose a reason for hiding this comment

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

"// a version to another of DD4hep; 2. length parameters unit is cm while PPS uses mm." change to:
"// a version to another of DD4hep; 2. length parameters unit is the DD4hep configured unit while PPS uses mm."

Copy link
Contributor Author

@ghugo83 ghugo83 Jan 4, 2021

Choose a reason for hiding this comment

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

done

const std::vector<double>& params() const { return m_params; } // default unit: mm from oldDD, cm from DD4hep
const std::vector<double>& params() const { return m_params; } // default unit: mm for oldDD, DD4hep unit for DD4hep
bool isABox() const { return m_isABox; }
const DiamondDimensions& getDiamondDimensions() const {
if (!isABox()) {
Expand Down Expand Up @@ -132,7 +132,7 @@ class DetGeomDesc {
bool m_isDD4hep;
Translation m_trans; // in mm
RotationMatrix m_rot;
std::vector<double> m_params; // default unit: mm from oldDD, cm from DD4hep
std::vector<double> m_params; // default unit: mm from oldDD, DD4hep unit for DD4hep
bool m_isABox;
DiamondDimensions m_diamondBoxParams; // in mm
std::string m_sensorType;
Expand Down
18 changes: 8 additions & 10 deletions Geometry/VeryForwardGeometryBuilder/src/DetGeomDesc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include "DataFormats/CTPPSDetId/interface/TotemTimingDetId.h"
#include "DataFormats/CTPPSDetId/interface/CTPPSPixelDetId.h"
#include "DataFormats/CTPPSDetId/interface/CTPPSDiamondDetId.h"
#include "DataFormats/Math/interface/GeantUnits.h"
#include <DD4hep/DD4hepUnits.h>
#include "FWCore/MessageLogger/interface/MessageLogger.h"

/*
Expand All @@ -47,14 +47,14 @@ DetGeomDesc::DetGeomDesc(const cms::DDFilteredView& fv, const bool isRun2)
: m_name(computeNameWithNoNamespace(fv.name())),
m_copy(fv.copyNum()),
m_isDD4hep(true),
m_trans(geant_units::operators::convertCmToMm(fv.translation())), // converted from cm (DD4hep) to mm
m_trans(fv.translation() / dd4hep::mm), // converted from DD4hep unit to mm
m_rot(fv.rotation()),
m_params(computeParameters(fv)), // default unit from DD4hep (cm)
m_params(computeParameters(fv)), // default unit from DD4hep
m_isABox(dd4hep::isA<dd4hep::Box>(fv.solid())),
m_diamondBoxParams(computeDiamondDimensions(m_isABox, m_isDD4hep, m_params)), // converted from cm (DD4hep) to mm
m_diamondBoxParams(computeDiamondDimensions(m_isABox, m_isDD4hep, m_params)), // converted from DD4hep unit to mm
m_sensorType(computeSensorType(fv.name())),
m_geographicalID(computeDetIDFromDD4hep(m_name, fv.copyNos(), fv.copyNum(), isRun2)),
m_z(geant_units::operators::convertCmToMm(fv.translation().z())) // converted from cm (DD4hep) to mm
m_z(fv.translation().z() / dd4hep::mm) // converted from DD4hep unit to mm
{}

DetGeomDesc::DetGeomDesc(const DetGeomDesc& ref, CopyMode cm) {
Expand Down Expand Up @@ -140,7 +140,7 @@ std::vector<double> DetGeomDesc::computeParameters(const cms::DDFilteredView& fv
* Compute diamond dimensions.
* The diamond sensors are represented by the Box shape parameters.
* oldDD: params are already in mm.
* DD4hep: convert params from cm (DD4hep) to mm (legacy expected by PPS reco software).
* DD4hep: convert params from DD4hep unit to mm (legacy expected by PPS reco software).
*/
DiamondDimensions DetGeomDesc::computeDiamondDimensions(const bool isABox,
const bool isDD4hep,
Expand All @@ -151,10 +151,8 @@ DiamondDimensions DetGeomDesc::computeDiamondDimensions(const bool isABox,
// mm (legacy)
boxShapeParameters = {params.at(0), params.at(1), params.at(2)};
} else {
// convert cm (DD4hep) to mm (legacy expected by PPS reco software)
boxShapeParameters = {geant_units::operators::convertCmToMm(params.at(0)),
geant_units::operators::convertCmToMm(params.at(1)),
geant_units::operators::convertCmToMm(params.at(2))};
// convert from DD4hep unit to mm (legacy expected by PPS reco software)
boxShapeParameters = {params.at(0) / dd4hep::mm, params.at(1) / dd4hep::mm, params.at(2) / dd4hep::mm};
}
}
return boxShapeParameters;
Expand Down