Skip to content

Commit

Permalink
Merge branch 'main' into gx2f-tempbackend
Browse files Browse the repository at this point in the history
  • Loading branch information
AJPfleger authored Jun 28, 2024
2 parents 734bdef + 430085b commit e725659
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 53 deletions.
35 changes: 35 additions & 0 deletions Core/include/Acts/Surfaces/SurfaceMergingException.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// This file is part of the Acts project.
//
// Copyright (C) 2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

#pragma once

#include <exception>
#include <memory>
#include <string>

namespace Acts {
class Surface;

/// @brief Exception type failures to merge two surfaces
class SurfaceMergingException : public std::exception {
public:
SurfaceMergingException(std::weak_ptr<const Surface> surfaceA,
std::weak_ptr<const Surface> surfaceB,
const std::string& reason)
: m_surfaceA(std::move(surfaceA)),
m_surfaceB(std::move(surfaceB)),
m_message(std::string{"Failure to merge surfaces: "} + reason) {}

const char* what() const throw() override { return m_message.c_str(); }

private:
std::weak_ptr<const Surface> m_surfaceA;
std::weak_ptr<const Surface> m_surfaceB;
std::string m_message;
};
} // namespace Acts
45 changes: 30 additions & 15 deletions Core/src/Surfaces/CylinderSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "Acts/Geometry/GeometryObject.hpp"
#include "Acts/Surfaces/CylinderBounds.hpp"
#include "Acts/Surfaces/SurfaceError.hpp"
#include "Acts/Surfaces/SurfaceMergingException.hpp"
#include "Acts/Surfaces/detail/AlignmentHelper.hpp"
#include "Acts/Surfaces/detail/FacesHelper.hpp"
#include "Acts/Surfaces/detail/MergeHelper.hpp"
Expand All @@ -28,6 +29,7 @@
#include <cmath>
#include <iostream>
#include <memory>
#include <stdexcept>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -378,16 +380,18 @@ std::shared_ptr<Acts::CylinderSurface> Acts::CylinderSurface::mergedWith(
// surface cannot have any relative rotation
if (!otherLocal.linear().isApprox(RotationMatrix3::Identity())) {
ACTS_ERROR("CylinderSurface::merge: surfaces have relative rotation");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"CylinderSurface::merge: surfaces have relative rotation");
}

auto checkNoBevel = [&logger](const auto& bounds) {
auto checkNoBevel = [this, &logger, &other](const auto& bounds) {
if (bounds.get(CylinderBounds::eBevelMinZ) != 0.0) {
ACTS_ERROR(
"CylinderVolumeStack requires all volumes to have a bevel angle of "
"0");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"CylinderVolumeStack requires all volumes to have a bevel angle of "
"0");
}
Expand All @@ -396,7 +400,8 @@ std::shared_ptr<Acts::CylinderSurface> Acts::CylinderSurface::mergedWith(
ACTS_ERROR(
"CylinderVolumeStack requires all volumes to have a bevel angle of "
"0");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"CylinderVolumeStack requires all volumes to have a bevel angle of "
"0");
}
Expand All @@ -409,7 +414,8 @@ std::shared_ptr<Acts::CylinderSurface> Acts::CylinderSurface::mergedWith(
if (std::abs(bounds().get(CylinderBounds::eR) -
other.bounds().get(CylinderBounds::eR)) > tolerance) {
ACTS_ERROR("CylinderSurface::merge: surfaces have different radii");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"CylinderSurface::merge: surfaces have different radii");
}

Expand All @@ -422,7 +428,8 @@ std::shared_ptr<Acts::CylinderSurface> Acts::CylinderSurface::mergedWith(
std::abs(translation[1]) > tolerance) {
ACTS_ERROR(
"CylinderSurface::merge: surfaces have relative translation in x/y");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"CylinderSurface::merge: surfaces have relative translation in x/y");
}

Expand All @@ -449,7 +456,8 @@ std::shared_ptr<Acts::CylinderSurface> Acts::CylinderSurface::mergedWith(
if (std::abs(maxZ - otherMinZ) > tolerance &&
std::abs(minZ - otherMaxZ) > tolerance) {
ACTS_ERROR("CylinderSurface::merge: surfaces have incompatible z bounds");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"CylinderSurface::merge: surfaces have incompatible z bounds");
}

Expand All @@ -473,7 +481,8 @@ std::shared_ptr<Acts::CylinderSurface> Acts::CylinderSurface::mergedWith(
ACTS_ERROR(
"CylinderSurface::merge: surfaces have relative translation in z for "
"rPhi merging");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"CylinderSurface::merge: surfaces have relative translation in z for "
"rPhi merging");
}
Expand All @@ -484,15 +493,21 @@ std::shared_ptr<Acts::CylinderSurface> Acts::CylinderSurface::mergedWith(
ActsScalar otherHlPhi = other.bounds().get(CylinderBounds::eHalfPhiSector);
ActsScalar otherAvgPhi = other.bounds().get(CylinderBounds::eAveragePhi);

auto [newHlPhi, newAvgPhi] = detail::mergedPhiSector(
hlPhi, avgPhi, otherHlPhi, otherAvgPhi, logger, tolerance);
try {
auto [newHlPhi, newAvgPhi] = detail::mergedPhiSector(
hlPhi, avgPhi, otherHlPhi, otherAvgPhi, logger, tolerance);

auto newBounds = std::make_shared<CylinderBounds>(
r, bounds().get(CylinderBounds::eHalfLengthZ), newHlPhi, newAvgPhi);
auto newBounds = std::make_shared<CylinderBounds>(
r, bounds().get(CylinderBounds::eHalfLengthZ), newHlPhi, newAvgPhi);

return Surface::makeShared<CylinderSurface>(transform(gctx), newBounds);
return Surface::makeShared<CylinderSurface>(transform(gctx), newBounds);
} catch (const std::invalid_argument& e) {
throw SurfaceMergingException(getSharedPtr(), other.getSharedPtr(),
e.what());
}
} else {
throw std::invalid_argument("CylinderSurface::merge: invalid direction " +
binningValueNames()[direction]);
throw SurfaceMergingException(getSharedPtr(), other.getSharedPtr(),
"CylinderSurface::merge: invalid direction " +
binningValueNames()[direction]);
}
}
43 changes: 29 additions & 14 deletions Core/src/Surfaces/DiscSurface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "Acts/Surfaces/RadialBounds.hpp"
#include "Acts/Surfaces/SurfaceBounds.hpp"
#include "Acts/Surfaces/SurfaceError.hpp"
#include "Acts/Surfaces/SurfaceMergingException.hpp"
#include "Acts/Surfaces/detail/FacesHelper.hpp"
#include "Acts/Surfaces/detail/MergeHelper.hpp"
#include "Acts/Surfaces/detail/PlanarHelper.hpp"
Expand Down Expand Up @@ -392,7 +393,8 @@ std::shared_ptr<Acts::DiscSurface> Acts::DiscSurface::mergedWith(
// surface cannot have any relative rotation
if (!otherLocal.linear().isApprox(RotationMatrix3::Identity())) {
ACTS_ERROR("DiscSurface::merge: surfaces have relative rotation");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: surfaces have relative rotation");
}

Expand All @@ -403,7 +405,8 @@ std::shared_ptr<Acts::DiscSurface> Acts::DiscSurface::mergedWith(
std::abs(translation[2]) > tolerance) {
ACTS_ERROR(
"DiscSurface::merge: surfaces have relative translation in x/y/z");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: surfaces have relative translation in x/y/z");
}

Expand All @@ -413,7 +416,8 @@ std::shared_ptr<Acts::DiscSurface> Acts::DiscSurface::mergedWith(

if (bounds == nullptr || otherBounds == nullptr) {
ACTS_ERROR("DiscSurface::merge: surfaces have bounds other than radial");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: surfaces have bounds other than radial");
}

Expand Down Expand Up @@ -447,19 +451,22 @@ std::shared_ptr<Acts::DiscSurface> Acts::DiscSurface::mergedWith(
if (std::abs(minR - otherMaxR) > tolerance &&
std::abs(maxR - otherMinR) > tolerance) {
ACTS_ERROR("DiscSurface::merge: surfaces are not touching r");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: surfaces are not touching in r");
}

if (std::abs(avgPhi - otherAvgPhi) > tolerance) {
ACTS_ERROR("DiscSurface::merge: surfaces have different average phi");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: surfaces have different average phi");
}

if (std::abs(hlPhi - otherHlPhi) > tolerance) {
ACTS_ERROR("DiscSurface::merge: surfaces have different half phi sector");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: surfaces have different half phi sector");
}

Expand All @@ -476,22 +483,30 @@ std::shared_ptr<Acts::DiscSurface> Acts::DiscSurface::mergedWith(
if (std::abs(maxR - otherMaxR) > tolerance ||
std::abs(minR - otherMinR) > tolerance) {
ACTS_ERROR("DiscSurface::merge: surfaces don't have same r bounds");
throw std::invalid_argument(
throw SurfaceMergingException(
getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: surfaces don't have same r bounds");
}

auto [newHlPhi, newAvgPhi] = detail::mergedPhiSector(
hlPhi, avgPhi, otherHlPhi, otherAvgPhi, logger, tolerance);
try {
auto [newHlPhi, newAvgPhi] = detail::mergedPhiSector(
hlPhi, avgPhi, otherHlPhi, otherAvgPhi, logger, tolerance);

auto newBounds =
std::make_shared<RadialBounds>(minR, maxR, newHlPhi, newAvgPhi);
auto newBounds =
std::make_shared<RadialBounds>(minR, maxR, newHlPhi, newAvgPhi);

return Surface::makeShared<DiscSurface>(transform(gctx), newBounds);
return Surface::makeShared<DiscSurface>(transform(gctx), newBounds);
} catch (const std::invalid_argument& e) {
throw SurfaceMergingException(getSharedPtr(), other.getSharedPtr(),
e.what());
}

} else {
ACTS_ERROR("DiscSurface::merge: invalid direction "
<< binningValueNames()[direction]);
throw std::invalid_argument("DiscSurface::merge: invalid direction " +
binningValueNames()[direction]);

throw SurfaceMergingException(getSharedPtr(), other.getSharedPtr(),
"DiscSurface::merge: invalid direction " +
binningValueNames()[direction]);
}
}
25 changes: 13 additions & 12 deletions Tests/UnitTests/Core/Surfaces/CylinderSurfaceTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "Acts/Surfaces/CylinderSurface.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Surfaces/SurfaceBounds.hpp"
#include "Acts/Surfaces/SurfaceMergingException.hpp"
#include "Acts/Tests/CommonHelpers/FloatComparisons.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Helpers.hpp"
Expand Down Expand Up @@ -352,60 +353,60 @@ BOOST_DATA_TEST_CASE(IncompatibleZDirection,
base * Translation3{Vector3::UnitZ() * 200_mm}, 30_mm, 100_mm);

BOOST_CHECK_THROW(cyl->mergedWith(testContext, *cyl2, Acts::binPhi, *logger),
std::invalid_argument);
SurfaceMergingException);

auto cylShiftedXy = Surface::makeShared<CylinderSurface>(
base * Translation3{Vector3{1_mm, 2_mm, 200_mm}}, 30_mm, 100_mm);
BOOST_CHECK_THROW(
cyl->mergedWith(testContext, *cylShiftedXy, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);

auto cylRotatedZ = Surface::makeShared<CylinderSurface>(
base * AngleAxis3{10_degree, Vector3::UnitZ()} *
Translation3{Vector3::UnitZ() * 200_mm},
30_mm, 100_mm);
BOOST_CHECK_THROW(
cyl->mergedWith(testContext, *cylRotatedZ, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);

auto cylRotatedX = Surface::makeShared<CylinderSurface>(
base * AngleAxis3{10_degree, Vector3::UnitX()} *
Translation3{Vector3::UnitZ() * 200_mm},
30_mm, 100_mm);
BOOST_CHECK_THROW(
cyl->mergedWith(testContext, *cylRotatedX, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);

// Cylinder with different radius
auto cyl3 = Surface::makeShared<CylinderSurface>(
base * Translation3{Vector3::UnitZ() * 200_mm}, 35_mm, 100_mm);
BOOST_CHECK_THROW(cyl->mergedWith(testContext, *cyl3, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);

// Cylinder with bevel
auto cyl4 = Surface::makeShared<CylinderSurface>(
base * Translation3{Vector3::UnitZ() * 200_mm}, 30_mm, 100_mm, M_PI, 0,
M_PI / 8.0);
BOOST_CHECK_THROW(cyl->mergedWith(testContext, *cyl4, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);

auto cyl5 = Surface::makeShared<CylinderSurface>(
base * Translation3{Vector3::UnitZ() * 200_mm}, 30_mm, 100_mm, M_PI, 0, 0,
M_PI / 8.0);
BOOST_CHECK_THROW(cyl->mergedWith(testContext, *cyl5, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);

// Cylinder with overlap in z
auto cyl6 = Surface::makeShared<CylinderSurface>(
base * Translation3{Vector3::UnitZ() * 150_mm}, 30_mm, 100_mm);
BOOST_CHECK_THROW(cyl->mergedWith(testContext, *cyl6, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);

// Cylinder with gap in z
auto cyl7 = Surface::makeShared<CylinderSurface>(
base * Translation3{Vector3::UnitZ() * 250_mm}, 30_mm, 100_mm);
BOOST_CHECK_THROW(cyl->mergedWith(testContext, *cyl7, Acts::binZ, *logger),
std::invalid_argument);
SurfaceMergingException);
}

BOOST_DATA_TEST_CASE(ZDirection,
Expand Down Expand Up @@ -465,22 +466,22 @@ BOOST_DATA_TEST_CASE(IncompatibleRPhiDirection,
45_degree, a(85_degree));
BOOST_CHECK_THROW(
cylPhi->mergedWith(testContext, *cylPhi2, Acts::binRPhi, *logger),
std::invalid_argument);
SurfaceMergingException);

// Cylinder with gap in phi
auto cylPhi3 = Surface::makeShared<CylinderSurface>(base, 30_mm, 100_mm,
45_degree, a(105_degree));
BOOST_CHECK_THROW(
cylPhi->mergedWith(testContext, *cylPhi3, Acts::binRPhi, *logger),
std::invalid_argument);
SurfaceMergingException);

// Cylinder with a z shift
auto cylPhi4 = Surface::makeShared<CylinderSurface>(
base * Translation3{Vector3::UnitZ() * 20_mm}, 30_mm, 100_mm, 45_degree,
a(95_degree));
BOOST_CHECK_THROW(
cylPhi->mergedWith(testContext, *cylPhi4, Acts::binRPhi, *logger),
std::invalid_argument);
SurfaceMergingException);
}

BOOST_DATA_TEST_CASE(RPhiDirection,
Expand Down
Loading

0 comments on commit e725659

Please sign in to comment.