From c82a101bc17c9baf3d6413288b10d73716660829 Mon Sep 17 00:00:00 2001
From: Scott Snyder <scott.snyder@cern.ch>
Date: Wed, 8 May 2024 09:43:39 +0200
Subject: [PATCH] MuonReadoutGeometryR4: Speed up handling of tube transforms.
MuonReadoutGeometryR4: Speed up handling of tube transforms.
Calling toTubeFrame is expensive, since it needs to walk over all children
until it gets to the desired value. Hence, calling toTubeFrame from within
a loop over tubes is an N^2 operation. Further, MdtReadoutElement::initElement()
was calling toTubeFrame multiple times for each tube.
This is especially painful in a debug build, where Eigen matrix operations
are very slow.
Speed up by using GeoVolumeCursor to change from the nested O(N^2) iteration
to a single O(N) iteration, and only evaluate the transform once per tube.
Also fix a logic error the prevented the layer-to-layer pitch test from
executing.
---
.../MuonReadoutGeometryR4/MdtTubeLayer.h | 5 ++++-
.../src/MdtReadoutElement.cxx | 15 +++++++++++----
.../MuonReadoutGeometryR4/src/MdtTubeLayer.cxx | 3 +++
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/MuonReadoutGeometryR4/MdtTubeLayer.h b/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/MuonReadoutGeometryR4/MdtTubeLayer.h
index b12665bd3515..d3b6f2948796 100644
--- a/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/MuonReadoutGeometryR4/MdtTubeLayer.h
+++ b/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/MuonReadoutGeometryR4/MdtTubeLayer.h
@@ -7,6 +7,7 @@
#include <MuonReadoutGeometryR4/MuonDetectorDefs.h>
#include <GeoModelKernel/GeoVPhysVol.h>
#include <GeoModelKernel/GeoTransform.h>
+#include <GeoModelKernel/GeoVolumeCursor.h>
#include <GeoModelUtilities/TransientConstSharedPtr.h>
#include <set>
@@ -40,6 +41,8 @@ namespace MuonGMR4{
unsigned int nTubes() const;
///@brief: Returns the transformation from the layer to the muon station
const Amg::Transform3D& layerTransform() const;
+ ///@brief Return a cursor object over the tubes in the layer.
+ GeoVolumeCursor tubeCursor() const;
///@brief Returns the transformation of the tube to the muon station
/// Index counting [0 - nTubes()-1]
const Amg::Transform3D tubeTransform(const unsigned int tube) const;
@@ -55,4 +58,4 @@ namespace MuonGMR4{
};
}
-#endif
\ No newline at end of file
+#endif
diff --git a/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtReadoutElement.cxx b/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtReadoutElement.cxx
index 9fa10513a9b8..a8d172e2f1ed 100644
--- a/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtReadoutElement.cxx
+++ b/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtReadoutElement.cxx
@@ -70,7 +70,11 @@ StatusCode MdtReadoutElement::initElement() {
#endif
/// Cache the transformations to the tube layers
std::optional<Amg::Vector3D> prevTubePos{std::nullopt};
- for (unsigned int tube = 1; tube <= numTubesInLay(); ++ tube) {
+ MdtTubeLayer& layer = *m_pars.tubeLayers[lay-1];
+ GeoVolumeCursor tubeCursor = layer.tubeCursor();
+ GeoTrf::Transform3D layerTransform = layer.layerTransform();
+ for (unsigned int tube = 1; tube <= numTubesInLay(); ++ tube, tubeCursor.next()) {
+ assert (!tubeCursor.atEnd());
const IdentifierHash idHash = measurementHash(lay,tube);
if (m_pars.removedTubes.count(idHash)) {
prevTubePos = std::nullopt;
@@ -81,15 +85,16 @@ StatusCode MdtReadoutElement::initElement() {
ATH_CHECK(strawSurfaceFactory(idHash, m_pars.tubeBounds->make_bounds(innerTubeRadius(), 0.5*tubeLength(idHash))));
#endif
///Ensure that all linear transformations are rotations
- const AmgSymMatrix(3) tubeRot = toTubeFrame(idHash).linear();
+ GeoTrf::Transform3D tubeFrame = layerTransform*tubeCursor.getDefTransform();
+ const AmgSymMatrix(3) tubeRot = tubeFrame.linear();
if (std::abs(tubeRot.determinant()- 1.) > std::numeric_limits<float>::epsilon()){
ATH_MSG_FATAL(__FILE__<<":"<<__LINE__<<" Transformation matrix is not a pure rotation for "<<
idHelperSvc()->toStringDetEl(identify())<<" in layer: "<<lay<<", tube: "<<tube
- <<Amg::toString(toTubeFrame(idHash)));
+ <<Amg::toString(tubeFrame));
return StatusCode::FAILURE;
}
/// Ensure that all tubes have the same pitch
- const Amg::Vector3D tubePos = toTubeFrame(idHash).translation();
+ const Amg::Vector3D tubePos = tubeFrame.translation();
constexpr double pitchTolerance = 20. * Gaudi::Units::micrometer;
if (prevTubePos) {
@@ -112,6 +117,8 @@ StatusCode MdtReadoutElement::initElement() {
<<dR<<" tube position: "<<Amg::toString(tubePos,2)
<<" previous:"<<Amg::toString((*prevLayPos), 2));
}
+ }
+ if (tube == 1) {
prevLayPos = std::make_optional<Amg::Vector3D>(tubePos);
}
prevTubePos = std::make_optional<Amg::Vector3D>(tubePos);
diff --git a/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtTubeLayer.cxx b/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtTubeLayer.cxx
index 1b7f205314db..a0daaef44f2d 100644
--- a/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtTubeLayer.cxx
+++ b/MuonSpectrometer/MuonPhaseII/MuonDetDescr/MuonReadoutGeometryR4/src/MdtTubeLayer.cxx
@@ -56,6 +56,9 @@ const Amg::Transform3D MdtTubeLayer::tubeTransform(const unsigned int tube) cons
m_layerNode->exec(&volAcc);
return layerTransform() * volAcc.getDefTransform();
}
+GeoVolumeCursor MdtTubeLayer::tubeCursor() const {
+ return GeoVolumeCursor(m_layerNode);
+}
const Amg::Vector3D MdtTubeLayer::tubePosInLayer(const unsigned int tube) const {
return layerTransform().inverse() * tubeTransform(tube).translation();
}
--
GitLab