From c0457bbed7d45d41f7a15f4816cded0c95d23cf5 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 23 Nov 2024 11:32:34 +0100 Subject: [PATCH 1/4] chore!: Clean some Material code --- .../Acts/Material/BinnedSurfaceMaterial.hpp | 8 ++--- .../Acts/Material/GridSurfaceMaterial.hpp | 12 +++---- .../Material/HomogeneousSurfaceMaterial.hpp | 30 ++++------------ .../Material/HomogeneousVolumeMaterial.hpp | 10 ------ .../Acts/Material/ISurfaceMaterial.hpp | 16 ++++----- Core/include/Acts/Material/Material.hpp | 18 +++++----- Core/include/Acts/Material/MaterialSlab.hpp | 14 ++++---- .../Acts/Material/ProtoSurfaceMaterial.hpp | 5 ++- Core/src/Material/BinnedSurfaceMaterial.cpp | 5 ++- .../Material/HomogeneousSurfaceMaterial.cpp | 36 ++++++++++++++----- .../Material/HomogeneousVolumeMaterial.cpp | 22 +++++++++--- Core/src/Material/Material.cpp | 35 +++++++++++------- Core/src/Material/MaterialSlab.cpp | 4 +++ .../Io/Root/src/RootMaterialTrackWriter.cpp | 3 -- .../Plugins/Json/MaterialJsonConverter.hpp | 2 +- .../Material/GridSurfaceMaterialTests.cpp | 8 ++--- .../HomogeneousSurfaceMaterialTests.cpp | 2 +- .../Core/Material/ISurfaceMaterialTests.cpp | 3 +- .../Core/Material/MaterialSlabTests.cpp | 2 -- 19 files changed, 119 insertions(+), 116 deletions(-) diff --git a/Core/include/Acts/Material/BinnedSurfaceMaterial.hpp b/Core/include/Acts/Material/BinnedSurfaceMaterial.hpp index 97d3347c9e1..d5534803d08 100644 --- a/Core/include/Acts/Material/BinnedSurfaceMaterial.hpp +++ b/Core/include/Acts/Material/BinnedSurfaceMaterial.hpp @@ -13,9 +13,7 @@ #include "Acts/Material/MaterialSlab.hpp" #include "Acts/Utilities/BinUtility.hpp" -#include #include -#include namespace Acts { @@ -83,10 +81,10 @@ class BinnedSurfaceMaterial : public ISurfaceMaterial { /// Destructor ~BinnedSurfaceMaterial() override = default; - /// Scale operator + /// Scale operation /// - /// @param scale is the scale factor for the full material - BinnedSurfaceMaterial& operator*=(double scale) final; + /// @param factor is the scale factor for the full material + BinnedSurfaceMaterial& scale(double factor) final; /// Return the BinUtility const BinUtility& binUtility() const; diff --git a/Core/include/Acts/Material/GridSurfaceMaterial.hpp b/Core/include/Acts/Material/GridSurfaceMaterial.hpp index 6c2d6045ca2..4d25881e512 100644 --- a/Core/include/Acts/Material/GridSurfaceMaterial.hpp +++ b/Core/include/Acts/Material/GridSurfaceMaterial.hpp @@ -11,10 +11,8 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Material/ISurfaceMaterial.hpp" #include "Acts/Material/MaterialSlab.hpp" -#include "Acts/Utilities/BinningType.hpp" #include "Acts/Utilities/Delegate.hpp" #include "Acts/Utilities/GridAccessHelpers.hpp" -#include "Acts/Utilities/VectorHelpers.hpp" #include #include @@ -44,7 +42,7 @@ struct GridMaterialAccessor { /// /// @note this is not particularly fast template - void scale(grid_type& grid, ActsScalar scale) { + void scale(grid_type& grid, double scale) { // Loop through the grid bins, get the indices and scale the material for (std::size_t ib = 0; ib < grid.size(); ++ib) { grid.at(ib).scaleThickness(scale); @@ -74,7 +72,7 @@ struct IndexedMaterialAccessor { /// /// @param scale the amount of the scaling template - void scale(grid_type& /*grid*/, ActsScalar scale) { + void scale(grid_type& /*grid*/, double scale) { for (auto& m : material) { m.scaleThickness(scale); } @@ -118,7 +116,7 @@ struct GloballyIndexedMaterialAccessor { /// outcome is unpredictable. /// template - void scale(grid_type& grid, ActsScalar scale) { + void scale(grid_type& grid, double scale) { if (sharedEntries) { throw std::invalid_argument( "GloballyIndexedMaterialAccessor: shared entry scaling is not " @@ -198,8 +196,8 @@ class GridSurfaceMaterialT : public ISurfaceMaterial { /// Scale operator /// /// @param scale is the scale factor applied - ISurfaceMaterial& operator*=(ActsScalar scale) final { - m_materialAccessor.scale(m_grid, scale); + ISurfaceMaterial& scale(double factor) final { + m_materialAccessor.scale(m_grid, factor); return (*this); } diff --git a/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp b/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp index 1ea14e632ac..dee99dae093 100644 --- a/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp +++ b/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp @@ -12,7 +12,6 @@ #include "Acts/Material/ISurfaceMaterial.hpp" #include "Acts/Material/MaterialSlab.hpp" -#include #include namespace Acts { @@ -59,17 +58,17 @@ class HomogeneousSurfaceMaterial : public ISurfaceMaterial { HomogeneousSurfaceMaterial& operator=(HomogeneousSurfaceMaterial&& hsm) = default; - /// Scale operator - /// - it is effectively a thickness scaling - /// - /// @param scale is the scale factor - HomogeneousSurfaceMaterial& operator*=(double scale) final; - /// Equality operator /// /// @param hsm is the source material bool operator==(const HomogeneousSurfaceMaterial& hsm) const; + /// Scale operator + /// - it is effectively a thickness scaling + /// + /// @param factor is the scale factor + HomogeneousSurfaceMaterial& scale(double factor) final; + /// @copydoc ISurfaceMaterial::materialSlab(const Vector2&) const /// /// @note the input parameter is ignored @@ -94,22 +93,7 @@ class HomogeneousSurfaceMaterial : public ISurfaceMaterial { private: /// The five different MaterialSlab - MaterialSlab m_fullMaterial = MaterialSlab(); + MaterialSlab m_fullMaterial; }; -inline const MaterialSlab& HomogeneousSurfaceMaterial::materialSlab( - const Vector2& /*lp*/) const { - return (m_fullMaterial); -} - -inline const MaterialSlab& HomogeneousSurfaceMaterial::materialSlab( - const Vector3& /*gp*/) const { - return (m_fullMaterial); -} - -inline bool HomogeneousSurfaceMaterial::operator==( - const HomogeneousSurfaceMaterial& hsm) const { - return (m_fullMaterial == hsm.m_fullMaterial); -} - } // namespace Acts diff --git a/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp b/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp index 91005dcae55..28226b24bed 100644 --- a/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp +++ b/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp @@ -67,14 +67,4 @@ class HomogeneousVolumeMaterial : public IVolumeMaterial { Material m_material = Material(); }; -inline const Material HomogeneousVolumeMaterial::material( - const Vector3& /*position*/) const { - return (m_material); -} - -inline bool HomogeneousVolumeMaterial::operator==( - const HomogeneousVolumeMaterial& hvm) const { - return (m_material == hvm.m_material); -} - } // namespace Acts diff --git a/Core/include/Acts/Material/ISurfaceMaterial.hpp b/Core/include/Acts/Material/ISurfaceMaterial.hpp index a379e18e715..457751a128b 100644 --- a/Core/include/Acts/Material/ISurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ISurfaceMaterial.hpp @@ -11,12 +11,9 @@ #include "Acts/Definitions/Algebra.hpp" #include "Acts/Definitions/Common.hpp" #include "Acts/Definitions/Direction.hpp" -#include "Acts/Geometry/GeometryIdentifier.hpp" #include "Acts/Material/MaterialSlab.hpp" -#include #include -#include namespace Acts { @@ -50,10 +47,10 @@ class ISurfaceMaterial { /// Destructor virtual ~ISurfaceMaterial() = default; - /// Scale operator + /// Scale material /// - /// @param scale is the scale factor applied - virtual ISurfaceMaterial& operator*=(double scale) = 0; + /// @param factor is the scale factor applied + virtual ISurfaceMaterial& scale(double factor) = 0; /// Return method for full material description of the Surface /// - from local coordinate on the surface @@ -128,9 +125,10 @@ class ISurfaceMaterial { } protected: - double m_splitFactor{1.}; //!< the split factor in favour of oppositePre - MappingType m_mappingType{ - Acts::MappingType::Default}; //!< Use the default mapping type by default + /// the split factor in favour of oppositePre + double m_splitFactor{1.}; + /// Use the default mapping type by default + MappingType m_mappingType{Acts::MappingType::Default}; }; inline double ISurfaceMaterial::factor(Direction pDir, diff --git a/Core/include/Acts/Material/Material.hpp b/Core/include/Acts/Material/Material.hpp index ae42e08cf68..c1993a2843f 100644 --- a/Core/include/Acts/Material/Material.hpp +++ b/Core/include/Acts/Material/Material.hpp @@ -8,11 +8,10 @@ #pragma once -#include "Acts/Definitions/Algebra.hpp" - #include #include -#include + +#include namespace Acts { @@ -82,6 +81,13 @@ class Material { Material& operator=(Material&& mat) = default; Material& operator=(const Material& mat) = default; + /// Check if two materials are exactly equal. + /// @note This is a strict equality check, i.e. the materials must + /// have identical properties. + /// @param mat is the material to compare to + /// @return true if the materials are equal + bool operator==(const Material& mat) const; + /// Check if the material is valid, i.e. it is not vacuum. bool isValid() const { return 0.0f < m_ar; } @@ -111,12 +117,6 @@ class Material { float m_ar = 0.0f; float m_z = 0.0f; float m_molarRho = 0.0f; - - friend constexpr bool operator==(const Material& lhs, const Material& rhs) { - return (lhs.m_x0 == rhs.m_x0) && (lhs.m_l0 == rhs.m_l0) && - (lhs.m_ar == rhs.m_ar) && (lhs.m_z == rhs.m_z) && - (lhs.m_molarRho == rhs.m_molarRho); - } }; std::ostream& operator<<(std::ostream& os, const Material& material); diff --git a/Core/include/Acts/Material/MaterialSlab.hpp b/Core/include/Acts/Material/MaterialSlab.hpp index 71c56c1a154..72e99da7718 100644 --- a/Core/include/Acts/Material/MaterialSlab.hpp +++ b/Core/include/Acts/Material/MaterialSlab.hpp @@ -58,6 +58,13 @@ class MaterialSlab { MaterialSlab& operator=(MaterialSlab&&) = default; MaterialSlab& operator=(const MaterialSlab&) = default; + /// Check if two materials are exactly equal. + /// @note This is a strict equality check, i.e. the materials must + /// have identical properties. + /// @param mat is the material to compare to + /// @return true if the materials are equal + bool operator==(const MaterialSlab& other) const; + /// Scale the material thickness by the given factor. void scaleThickness(float scale); @@ -78,13 +85,6 @@ class MaterialSlab { float m_thickness = 0.0f; float m_thicknessInX0 = 0.0f; float m_thicknessInL0 = 0.0f; - - friend constexpr bool operator==(const MaterialSlab& lhs, - const MaterialSlab& rhs) { - // t/X0 and t/L0 are dependent variables and need not be checked - return (lhs.m_material == rhs.m_material) && - (lhs.m_thickness == rhs.m_thickness); - } }; std::ostream& operator<<(std::ostream& os, const MaterialSlab& materialSlab); diff --git a/Core/include/Acts/Material/ProtoSurfaceMaterial.hpp b/Core/include/Acts/Material/ProtoSurfaceMaterial.hpp index 58dee9d01c7..8b6dfed3287 100644 --- a/Core/include/Acts/Material/ProtoSurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ProtoSurfaceMaterial.hpp @@ -14,7 +14,6 @@ #include "Acts/Material/MaterialSlab.hpp" #include "Acts/Utilities/BinUtility.hpp" -#include #include namespace Acts { @@ -67,9 +66,9 @@ class ProtoSurfaceMaterialT : public ISurfaceMaterial { ProtoSurfaceMaterialT& operator=( ProtoSurfaceMaterialT&& smproxy) = default; - /// Scale operator - dummy implementation + /// Scale operation - dummy implementation /// - ProtoSurfaceMaterialT& operator*=(double /*scale*/) final { + ProtoSurfaceMaterialT& scale(double /*factor*/) final { return (*this); } diff --git a/Core/src/Material/BinnedSurfaceMaterial.cpp b/Core/src/Material/BinnedSurfaceMaterial.cpp index 0113e89a6e3..aae46bd02d7 100644 --- a/Core/src/Material/BinnedSurfaceMaterial.cpp +++ b/Core/src/Material/BinnedSurfaceMaterial.cpp @@ -29,11 +29,10 @@ Acts::BinnedSurfaceMaterial::BinnedSurfaceMaterial( m_binUtility(binUtility), m_fullMaterial(std::move(fullProperties)) {} -Acts::BinnedSurfaceMaterial& Acts::BinnedSurfaceMaterial::operator*=( - double scale) { +Acts::BinnedSurfaceMaterial& Acts::BinnedSurfaceMaterial::scale(double factor) { for (auto& materialVector : m_fullMaterial) { for (auto& materialBin : materialVector) { - materialBin.scaleThickness(scale); + materialBin.scaleThickness(factor); } } return (*this); diff --git a/Core/src/Material/HomogeneousSurfaceMaterial.cpp b/Core/src/Material/HomogeneousSurfaceMaterial.cpp index ec56cfd4c81..7ea0009a299 100644 --- a/Core/src/Material/HomogeneousSurfaceMaterial.cpp +++ b/Core/src/Material/HomogeneousSurfaceMaterial.cpp @@ -12,20 +12,38 @@ #include -Acts::HomogeneousSurfaceMaterial::HomogeneousSurfaceMaterial( - const MaterialSlab& full, double splitFactor, Acts::MappingType mappingType) +namespace Acts { + +HomogeneousSurfaceMaterial::HomogeneousSurfaceMaterial(const MaterialSlab& full, + double splitFactor, + MappingType mappingType) : ISurfaceMaterial(splitFactor, mappingType), m_fullMaterial(full) {} -Acts::HomogeneousSurfaceMaterial& Acts::HomogeneousSurfaceMaterial::operator*=( - double scale) { - m_fullMaterial.scaleThickness(scale); - return (*this); +bool HomogeneousSurfaceMaterial::operator==( + const HomogeneousSurfaceMaterial& hsm) const { + return m_fullMaterial == hsm.m_fullMaterial; +} + +HomogeneousSurfaceMaterial& HomogeneousSurfaceMaterial::scale(double factor) { + m_fullMaterial.scaleThickness(factor); + return *this; +} + +const MaterialSlab& HomogeneousSurfaceMaterial::materialSlab( + const Vector2& /*lp*/) const { + return m_fullMaterial; +} + +const MaterialSlab& HomogeneousSurfaceMaterial::materialSlab( + const Vector3& /*gp*/) const { + return m_fullMaterial; } -std::ostream& Acts::HomogeneousSurfaceMaterial::toStream( - std::ostream& sl) const { - sl << "Acts::HomogeneousSurfaceMaterial : " << std::endl; +std::ostream& HomogeneousSurfaceMaterial::toStream(std::ostream& sl) const { + sl << "HomogeneousSurfaceMaterial : " << std::endl; sl << " - fullMaterial : " << m_fullMaterial << std::endl; sl << " - split factor : " << m_splitFactor << std::endl; return sl; } + +} // namespace Acts diff --git a/Core/src/Material/HomogeneousVolumeMaterial.cpp b/Core/src/Material/HomogeneousVolumeMaterial.cpp index 574859bb648..47932919885 100644 --- a/Core/src/Material/HomogeneousVolumeMaterial.cpp +++ b/Core/src/Material/HomogeneousVolumeMaterial.cpp @@ -12,13 +12,25 @@ #include -Acts::HomogeneousVolumeMaterial::HomogeneousVolumeMaterial( - const Material& material) +namespace Acts { + +HomogeneousVolumeMaterial::HomogeneousVolumeMaterial(const Material& material) : m_material(material) {} -std::ostream& Acts::HomogeneousVolumeMaterial::toStream( - std::ostream& sl) const { - sl << "Acts::HomogeneousVolumeMaterial : " << std::endl; +bool HomogeneousVolumeMaterial::operator==( + const HomogeneousVolumeMaterial& hvm) const { + return m_material == hvm.m_material; +} + +const Material HomogeneousVolumeMaterial::material( + const Vector3& /*position*/) const { + return m_material; +} + +std::ostream& HomogeneousVolumeMaterial::toStream(std::ostream& sl) const { + sl << "HomogeneousVolumeMaterial : " << std::endl; sl << " - material : " << m_material << std::endl; return sl; } + +} // namespace Acts diff --git a/Core/src/Material/Material.cpp b/Core/src/Material/Material.cpp index dd8c1e5cca1..7a4deedb129 100644 --- a/Core/src/Material/Material.cpp +++ b/Core/src/Material/Material.cpp @@ -13,6 +13,8 @@ #include #include +namespace Acts { + namespace { enum MaterialClassificationNumberIndices { eRadiationLength = 0, @@ -23,12 +25,12 @@ enum MaterialClassificationNumberIndices { }; // Avogadro constant -constexpr double kAvogadro = 6.02214076e23 / Acts::UnitConstants::mol; +constexpr double kAvogadro = 6.02214076e23 / UnitConstants::mol; } // namespace -Acts::Material Acts::Material::fromMassDensity(float x0, float l0, float ar, - float z, float massRho) { - using namespace Acts::UnitLiterals; +Material Material::fromMassDensity(float x0, float l0, float ar, float z, + float massRho) { + using namespace UnitLiterals; Material mat; mat.m_x0 = x0; @@ -51,8 +53,8 @@ Acts::Material Acts::Material::fromMassDensity(float x0, float l0, float ar, return mat; } -Acts::Material Acts::Material::fromMolarDensity(float x0, float l0, float ar, - float z, float molarRho) { +Material Material::fromMolarDensity(float x0, float l0, float ar, float z, + float molarRho) { Material mat; mat.m_x0 = x0; mat.m_l0 = l0; @@ -62,15 +64,15 @@ Acts::Material Acts::Material::fromMolarDensity(float x0, float l0, float ar, return mat; } -Acts::Material::Material(const ParametersVector& parameters) +Material::Material(const ParametersVector& parameters) : m_x0(parameters[eRadiationLength]), m_l0(parameters[eInteractionLength]), m_ar(parameters[eRelativeAtomicMass]), m_z(parameters[eNuclearCharge]), m_molarRho(parameters[eMolarDensity]) {} -float Acts::Material::massDensity() const { - using namespace Acts::UnitLiterals; +float Material::massDensity() const { + using namespace UnitLiterals; // perform computations in double precision to avoid loss of precision const double atomicMass = static_cast(m_ar) * 1_u; @@ -78,14 +80,19 @@ float Acts::Material::massDensity() const { return atomicMass * numberDensity; } -float Acts::Material::meanExcitationEnergy() const { - using namespace Acts::UnitLiterals; +bool Material::operator==(const Material& mat) const { + return m_x0 == mat.m_x0 && m_l0 == mat.m_l0 && m_ar == mat.m_ar && + m_z == mat.m_z && m_molarRho == mat.m_molarRho; +} + +float Material::meanExcitationEnergy() const { + using namespace UnitLiterals; // use approximative computation as defined in ATL-SOFT-PUB-2008-003 return 16_eV * std::pow(m_z, 0.9f); } -Acts::Material::ParametersVector Acts::Material::parameters() const { +Material::ParametersVector Material::parameters() const { ParametersVector parameters; parameters[eRadiationLength] = m_x0; parameters[eInteractionLength] = m_l0; @@ -95,7 +102,7 @@ Acts::Material::ParametersVector Acts::Material::parameters() const { return parameters; } -std::ostream& Acts::operator<<(std::ostream& os, const Material& material) { +std::ostream& operator<<(std::ostream& os, const Material& material) { if (!material.isValid()) { os << "vacuum"; } else { @@ -107,3 +114,5 @@ std::ostream& Acts::operator<<(std::ostream& os, const Material& material) { } return os; } + +} // namespace Acts diff --git a/Core/src/Material/MaterialSlab.cpp b/Core/src/Material/MaterialSlab.cpp index acfd7064936..698ee8fe58c 100644 --- a/Core/src/Material/MaterialSlab.cpp +++ b/Core/src/Material/MaterialSlab.cpp @@ -33,6 +33,10 @@ MaterialSlab::MaterialSlab(const Material& material, float thickness) } } +bool MaterialSlab::operator==(const MaterialSlab& other) const { + return m_material == other.m_material && m_thickness == other.m_thickness; +} + MaterialSlab MaterialSlab::averageLayers(const MaterialSlab& layerA, const MaterialSlab& layerB) { return detail::combineSlabs(layerA, layerB); diff --git a/Examples/Io/Root/src/RootMaterialTrackWriter.cpp b/Examples/Io/Root/src/RootMaterialTrackWriter.cpp index a7a5ee39f8c..bec096d2ca7 100644 --- a/Examples/Io/Root/src/RootMaterialTrackWriter.cpp +++ b/Examples/Io/Root/src/RootMaterialTrackWriter.cpp @@ -10,7 +10,6 @@ #include "Acts/Geometry/GeometryIdentifier.hpp" #include "Acts/Geometry/TrackingVolume.hpp" -#include "Acts/Geometry/Volume.hpp" #include "Acts/Material/Material.hpp" #include "Acts/Material/MaterialInteraction.hpp" #include "Acts/Material/MaterialSlab.hpp" @@ -23,11 +22,9 @@ #include "Acts/Utilities/VectorHelpers.hpp" #include "ActsExamples/Framework/AlgorithmContext.hpp" -#include #include #include #include -#include #include #include diff --git a/Plugins/Json/include/Acts/Plugins/Json/MaterialJsonConverter.hpp b/Plugins/Json/include/Acts/Plugins/Json/MaterialJsonConverter.hpp index 1c49376e421..3aa1ddac0af 100644 --- a/Plugins/Json/include/Acts/Plugins/Json/MaterialJsonConverter.hpp +++ b/Plugins/Json/include/Acts/Plugins/Json/MaterialJsonConverter.hpp @@ -8,7 +8,6 @@ #pragma once -#include "Acts/Definitions/Algebra.hpp" #include "Acts/Material/ISurfaceMaterial.hpp" #include "Acts/Material/IVolumeMaterial.hpp" #include "Acts/Material/Material.hpp" @@ -21,6 +20,7 @@ // can not match our naming guidelines. namespace Acts { +class Surface; class ISurfaceMaterial; class IVolumeMaterial; class BinUtility; diff --git a/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp b/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp index fbce80b8d5e..c66df1d6027 100644 --- a/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp +++ b/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp @@ -178,7 +178,7 @@ BOOST_AUTO_TEST_CASE(GridIndexedMaterial1D) { BOOST_CHECK_EQUAL(ml4.material().X0(), 21.); // Now scale it - and access again - ism *= 2.; + ism.scale(2.); const Acts::MaterialSlab& sml0 = ism.materialSlab(l0); const Acts::MaterialSlab& sml1 = ism.materialSlab(l1); const Acts::MaterialSlab& sml2 = ism.materialSlab(l2); @@ -337,7 +337,7 @@ BOOST_AUTO_TEST_CASE(GridGloballyIndexedMaterialNonShared) { BOOST_CHECK_EQUAL(ml0g1.material().X0(), 31.); // Scale - ism1 *= 2.; + ism1.scale(2.); const Acts::MaterialSlab& sml0g1 = ism1.materialSlab(l0g1); BOOST_CHECK_EQUAL(sml0g1.thickness(), 8.); @@ -400,7 +400,7 @@ BOOST_AUTO_TEST_CASE(GridGloballyIndexedMaterialShared) { BOOST_CHECK_EQUAL(ml0g1.material().X0(), 1.); // scaling shared material should throw a std::invalid_argument - BOOST_CHECK_THROW(ism1 *= 2., std::invalid_argument); + BOOST_CHECK_THROW(ism1.scale(2.), std::invalid_argument); } // This test covers the grid material (non-indexed accessor) @@ -472,7 +472,7 @@ BOOST_AUTO_TEST_CASE(GridSurfaceMaterialTests) { BOOST_CHECK_EQUAL(ml4.thickness(), 4.); // Now scale it - and access again - gsm *= 2.; + gsm.scale(2.); const Acts::MaterialSlab& sml0 = gsm.materialSlab(l0); const Acts::MaterialSlab& sml1 = gsm.materialSlab(l1); diff --git a/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp b/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp index 7c310b7c11a..10db0c4b689 100644 --- a/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp +++ b/Tests/UnitTests/Core/Material/HomogeneousSurfaceMaterialTests.cpp @@ -53,7 +53,7 @@ BOOST_AUTO_TEST_CASE(HomogeneousSurfaceMaterial_scaling_test) { matHalf.scaleThickness(0.5); HomogeneousSurfaceMaterial hsm(mat, 1.); - hsm *= 0.5; + hsm.scale(0.5); auto matBin = hsm.materialSlab(Vector3(0., 0., 0.)); diff --git a/Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp b/Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp index 067e460b71f..1bdb2cef716 100644 --- a/Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp +++ b/Tests/UnitTests/Core/Material/ISurfaceMaterialTests.cpp @@ -14,7 +14,6 @@ #include "Acts/Material/ISurfaceMaterial.hpp" #include "Acts/Material/MaterialSlab.hpp" -#include #include namespace Acts::Test { @@ -22,7 +21,7 @@ namespace Acts::Test { class SurfaceMaterialStub : public ISurfaceMaterial { using ISurfaceMaterial::ISurfaceMaterial; - ISurfaceMaterial& operator*=(double /*scale*/) override { return *this; }; + ISurfaceMaterial& scale(double /*factor*/) override { return *this; }; const MaterialSlab& materialSlab(const Vector2& /*lp*/) const override { return m_fullMaterial; diff --git a/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp b/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp index 3dfab084a2b..677277ab060 100644 --- a/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp +++ b/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp @@ -70,8 +70,6 @@ BOOST_AUTO_TEST_CASE(scale_thickness) { Acts::MaterialSlab halfScaled = mat; halfScaled.scaleThickness(0.5); - BOOST_CHECK_NE(mat, halfMat); - BOOST_CHECK_EQUAL(halfMat, halfScaled); CHECK_CLOSE_REL(mat.thicknessInX0(), 2 * halfMat.thicknessInX0(), eps); CHECK_CLOSE_REL(mat.thicknessInL0(), 2 * halfMat.thicknessInL0(), eps); CHECK_CLOSE_REL(mat.thickness() * mat.material().massDensity(), From af7baef78050a1de82007b10c01daa278315628f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 23 Nov 2024 11:36:34 +0100 Subject: [PATCH 2/4] fix doc --- Core/include/Acts/Material/GridSurfaceMaterial.hpp | 2 +- Core/include/Acts/Material/MaterialSlab.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Core/include/Acts/Material/GridSurfaceMaterial.hpp b/Core/include/Acts/Material/GridSurfaceMaterial.hpp index 4d25881e512..d27912c85f3 100644 --- a/Core/include/Acts/Material/GridSurfaceMaterial.hpp +++ b/Core/include/Acts/Material/GridSurfaceMaterial.hpp @@ -195,7 +195,7 @@ class GridSurfaceMaterialT : public ISurfaceMaterial { /// Scale operator /// - /// @param scale is the scale factor applied + /// @param factor is the scale factor applied ISurfaceMaterial& scale(double factor) final { m_materialAccessor.scale(m_grid, factor); return (*this); diff --git a/Core/include/Acts/Material/MaterialSlab.hpp b/Core/include/Acts/Material/MaterialSlab.hpp index 72e99da7718..33fa8e8920e 100644 --- a/Core/include/Acts/Material/MaterialSlab.hpp +++ b/Core/include/Acts/Material/MaterialSlab.hpp @@ -61,7 +61,7 @@ class MaterialSlab { /// Check if two materials are exactly equal. /// @note This is a strict equality check, i.e. the materials must /// have identical properties. - /// @param mat is the material to compare to + /// @param other is the material to compare to /// @return true if the materials are equal bool operator==(const MaterialSlab& other) const; From 1938235d9d277c75bee5e3c6eadb9bf7aa04fb67 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 24 Nov 2024 10:07:41 +0100 Subject: [PATCH 3/4] pr feedback --- .../Material/HomogeneousSurfaceMaterial.hpp | 15 ++++++++++----- .../Material/HomogeneousVolumeMaterial.hpp | 17 +++++++++++------ .../Acts/Material/ISurfaceMaterial.hpp | 1 + Core/include/Acts/Material/Material.hpp | 18 +++++++++++------- Core/include/Acts/Material/MaterialSlab.hpp | 19 ++++++++++++------- .../Material/HomogeneousSurfaceMaterial.cpp | 5 ----- .../Material/HomogeneousVolumeMaterial.cpp | 5 ----- Core/src/Material/Material.cpp | 5 ----- Core/src/Material/MaterialSlab.cpp | 5 ----- .../Core/Material/MaterialSlabTests.cpp | 2 ++ 10 files changed, 47 insertions(+), 45 deletions(-) diff --git a/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp b/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp index dee99dae093..d3d2c2f6733 100644 --- a/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp +++ b/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp @@ -58,11 +58,6 @@ class HomogeneousSurfaceMaterial : public ISurfaceMaterial { HomogeneousSurfaceMaterial& operator=(HomogeneousSurfaceMaterial&& hsm) = default; - /// Equality operator - /// - /// @param hsm is the source material - bool operator==(const HomogeneousSurfaceMaterial& hsm) const; - /// Scale operator /// - it is effectively a thickness scaling /// @@ -94,6 +89,16 @@ class HomogeneousSurfaceMaterial : public ISurfaceMaterial { private: /// The five different MaterialSlab MaterialSlab m_fullMaterial; + + /// Check if two materials are exactly equal. + /// @note This is a strict equality check, i.e. the materials must + /// have identical properties. + /// @param other is the material to compare to + /// @return true if the materials are equal + friend constexpr bool operator==(const HomogeneousSurfaceMaterial& lhs, + const HomogeneousSurfaceMaterial& rhs) { + return lhs.m_fullMaterial == rhs.m_fullMaterial; + } }; } // namespace Acts diff --git a/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp b/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp index 28226b24bed..5136c2139b5 100644 --- a/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp +++ b/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp @@ -46,11 +46,6 @@ class HomogeneousVolumeMaterial : public IVolumeMaterial { HomogeneousVolumeMaterial& operator=(const HomogeneousVolumeMaterial& hvm) = default; - /// Equality operator - /// - /// @param hvm is the source material - bool operator==(const HomogeneousVolumeMaterial& hvm) const; - /// Access to actual material /// /// @param position is the request position for the material call @@ -64,7 +59,17 @@ class HomogeneousVolumeMaterial : public IVolumeMaterial { std::ostream& toStream(std::ostream& sl) const final; private: - Material m_material = Material(); + Material m_material; + + /// Check if two materials are exactly equal. + /// @note This is a strict equality check, i.e. the materials must + /// have identical properties. + /// @param other is the material to compare to + /// @return true if the materials are equal + friend constexpr bool operator==(const HomogeneousVolumeMaterial& lhs, + const HomogeneousVolumeMaterial& rhs) { + return lhs.m_material == rhs.m_material; + } }; } // namespace Acts diff --git a/Core/include/Acts/Material/ISurfaceMaterial.hpp b/Core/include/Acts/Material/ISurfaceMaterial.hpp index 457751a128b..cad6832441a 100644 --- a/Core/include/Acts/Material/ISurfaceMaterial.hpp +++ b/Core/include/Acts/Material/ISurfaceMaterial.hpp @@ -127,6 +127,7 @@ class ISurfaceMaterial { protected: /// the split factor in favour of oppositePre double m_splitFactor{1.}; + /// Use the default mapping type by default MappingType m_mappingType{Acts::MappingType::Default}; }; diff --git a/Core/include/Acts/Material/Material.hpp b/Core/include/Acts/Material/Material.hpp index c1993a2843f..41119d1df37 100644 --- a/Core/include/Acts/Material/Material.hpp +++ b/Core/include/Acts/Material/Material.hpp @@ -81,13 +81,6 @@ class Material { Material& operator=(Material&& mat) = default; Material& operator=(const Material& mat) = default; - /// Check if two materials are exactly equal. - /// @note This is a strict equality check, i.e. the materials must - /// have identical properties. - /// @param mat is the material to compare to - /// @return true if the materials are equal - bool operator==(const Material& mat) const; - /// Check if the material is valid, i.e. it is not vacuum. bool isValid() const { return 0.0f < m_ar; } @@ -117,6 +110,17 @@ class Material { float m_ar = 0.0f; float m_z = 0.0f; float m_molarRho = 0.0f; + + /// Check if two materials are exactly equal. + /// @note This is a strict equality check, i.e. the materials must + /// have identical properties. + /// @param mat is the material to compare to + /// @return true if the materials are equal + friend constexpr bool operator==(const Material& lhs, const Material& rhs) { + return (lhs.m_x0 == rhs.m_x0) && (lhs.m_l0 == rhs.m_l0) && + (lhs.m_ar == rhs.m_ar) && (lhs.m_z == rhs.m_z) && + (lhs.m_molarRho == rhs.m_molarRho); + } }; std::ostream& operator<<(std::ostream& os, const Material& material); diff --git a/Core/include/Acts/Material/MaterialSlab.hpp b/Core/include/Acts/Material/MaterialSlab.hpp index 33fa8e8920e..e94e063e9cc 100644 --- a/Core/include/Acts/Material/MaterialSlab.hpp +++ b/Core/include/Acts/Material/MaterialSlab.hpp @@ -58,13 +58,6 @@ class MaterialSlab { MaterialSlab& operator=(MaterialSlab&&) = default; MaterialSlab& operator=(const MaterialSlab&) = default; - /// Check if two materials are exactly equal. - /// @note This is a strict equality check, i.e. the materials must - /// have identical properties. - /// @param other is the material to compare to - /// @return true if the materials are equal - bool operator==(const MaterialSlab& other) const; - /// Scale the material thickness by the given factor. void scaleThickness(float scale); @@ -85,6 +78,18 @@ class MaterialSlab { float m_thickness = 0.0f; float m_thicknessInX0 = 0.0f; float m_thicknessInL0 = 0.0f; + + /// Check if two materials are exactly equal. + /// @note This is a strict equality check, i.e. the materials must + /// have identical properties. + /// @param other is the material to compare to + /// @return true if the materials are equal + friend constexpr bool operator==(const MaterialSlab& lhs, + const MaterialSlab& rhs) { + // t/X0 and t/L0 are dependent variables and need not be checked + return (lhs.m_material == rhs.m_material) && + (lhs.m_thickness == rhs.m_thickness); + } }; std::ostream& operator<<(std::ostream& os, const MaterialSlab& materialSlab); diff --git a/Core/src/Material/HomogeneousSurfaceMaterial.cpp b/Core/src/Material/HomogeneousSurfaceMaterial.cpp index 7ea0009a299..c3bef1f9de3 100644 --- a/Core/src/Material/HomogeneousSurfaceMaterial.cpp +++ b/Core/src/Material/HomogeneousSurfaceMaterial.cpp @@ -19,11 +19,6 @@ HomogeneousSurfaceMaterial::HomogeneousSurfaceMaterial(const MaterialSlab& full, MappingType mappingType) : ISurfaceMaterial(splitFactor, mappingType), m_fullMaterial(full) {} -bool HomogeneousSurfaceMaterial::operator==( - const HomogeneousSurfaceMaterial& hsm) const { - return m_fullMaterial == hsm.m_fullMaterial; -} - HomogeneousSurfaceMaterial& HomogeneousSurfaceMaterial::scale(double factor) { m_fullMaterial.scaleThickness(factor); return *this; diff --git a/Core/src/Material/HomogeneousVolumeMaterial.cpp b/Core/src/Material/HomogeneousVolumeMaterial.cpp index 47932919885..a5ea578f0ce 100644 --- a/Core/src/Material/HomogeneousVolumeMaterial.cpp +++ b/Core/src/Material/HomogeneousVolumeMaterial.cpp @@ -17,11 +17,6 @@ namespace Acts { HomogeneousVolumeMaterial::HomogeneousVolumeMaterial(const Material& material) : m_material(material) {} -bool HomogeneousVolumeMaterial::operator==( - const HomogeneousVolumeMaterial& hvm) const { - return m_material == hvm.m_material; -} - const Material HomogeneousVolumeMaterial::material( const Vector3& /*position*/) const { return m_material; diff --git a/Core/src/Material/Material.cpp b/Core/src/Material/Material.cpp index 7a4deedb129..29d9502fde9 100644 --- a/Core/src/Material/Material.cpp +++ b/Core/src/Material/Material.cpp @@ -80,11 +80,6 @@ float Material::massDensity() const { return atomicMass * numberDensity; } -bool Material::operator==(const Material& mat) const { - return m_x0 == mat.m_x0 && m_l0 == mat.m_l0 && m_ar == mat.m_ar && - m_z == mat.m_z && m_molarRho == mat.m_molarRho; -} - float Material::meanExcitationEnergy() const { using namespace UnitLiterals; diff --git a/Core/src/Material/MaterialSlab.cpp b/Core/src/Material/MaterialSlab.cpp index 698ee8fe58c..4fad731c6c1 100644 --- a/Core/src/Material/MaterialSlab.cpp +++ b/Core/src/Material/MaterialSlab.cpp @@ -11,7 +11,6 @@ #include "Acts/Material/detail/AverageMaterials.hpp" #include -#include #include #include @@ -33,10 +32,6 @@ MaterialSlab::MaterialSlab(const Material& material, float thickness) } } -bool MaterialSlab::operator==(const MaterialSlab& other) const { - return m_material == other.m_material && m_thickness == other.m_thickness; -} - MaterialSlab MaterialSlab::averageLayers(const MaterialSlab& layerA, const MaterialSlab& layerB) { return detail::combineSlabs(layerA, layerB); diff --git a/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp b/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp index 677277ab060..3dfab084a2b 100644 --- a/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp +++ b/Tests/UnitTests/Core/Material/MaterialSlabTests.cpp @@ -70,6 +70,8 @@ BOOST_AUTO_TEST_CASE(scale_thickness) { Acts::MaterialSlab halfScaled = mat; halfScaled.scaleThickness(0.5); + BOOST_CHECK_NE(mat, halfMat); + BOOST_CHECK_EQUAL(halfMat, halfScaled); CHECK_CLOSE_REL(mat.thicknessInX0(), 2 * halfMat.thicknessInX0(), eps); CHECK_CLOSE_REL(mat.thicknessInL0(), 2 * halfMat.thicknessInL0(), eps); CHECK_CLOSE_REL(mat.thickness() * mat.material().massDensity(), From c701aab40dce18bea91b90fdaa314c6bbf795a58 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 25 Nov 2024 09:21:27 +0100 Subject: [PATCH 4/4] fix docs --- .../Acts/Material/HomogeneousSurfaceMaterial.hpp | 12 ++++++++---- .../Acts/Material/HomogeneousVolumeMaterial.hpp | 12 ++++++++---- Core/include/Acts/Material/Material.hpp | 12 ++++++++---- Core/include/Acts/Material/MaterialSlab.hpp | 12 ++++++++---- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp b/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp index d3d2c2f6733..37cad2a18d6 100644 --- a/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp +++ b/Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp @@ -90,10 +90,14 @@ class HomogeneousSurfaceMaterial : public ISurfaceMaterial { /// The five different MaterialSlab MaterialSlab m_fullMaterial; - /// Check if two materials are exactly equal. - /// @note This is a strict equality check, i.e. the materials must - /// have identical properties. - /// @param other is the material to compare to + /// @brief Check if two materials are exactly equal. + /// + /// This is a strict equality check, i.e. the materials must have identical + /// properties. + /// + /// @param lhs is the left hand side material + /// @param rhs is the right hand side material + /// /// @return true if the materials are equal friend constexpr bool operator==(const HomogeneousSurfaceMaterial& lhs, const HomogeneousSurfaceMaterial& rhs) { diff --git a/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp b/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp index 5136c2139b5..2034387e764 100644 --- a/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp +++ b/Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp @@ -61,10 +61,14 @@ class HomogeneousVolumeMaterial : public IVolumeMaterial { private: Material m_material; - /// Check if two materials are exactly equal. - /// @note This is a strict equality check, i.e. the materials must - /// have identical properties. - /// @param other is the material to compare to + /// @brief Check if two materials are exactly equal. + /// + /// This is a strict equality check, i.e. the materials must have identical + /// properties. + /// + /// @param lhs is the left hand side material + /// @param rhs is the right hand side material + /// /// @return true if the materials are equal friend constexpr bool operator==(const HomogeneousVolumeMaterial& lhs, const HomogeneousVolumeMaterial& rhs) { diff --git a/Core/include/Acts/Material/Material.hpp b/Core/include/Acts/Material/Material.hpp index 41119d1df37..1caf59e97ec 100644 --- a/Core/include/Acts/Material/Material.hpp +++ b/Core/include/Acts/Material/Material.hpp @@ -111,10 +111,14 @@ class Material { float m_z = 0.0f; float m_molarRho = 0.0f; - /// Check if two materials are exactly equal. - /// @note This is a strict equality check, i.e. the materials must - /// have identical properties. - /// @param mat is the material to compare to + /// @brief Check if two materials are exactly equal. + /// + /// This is a strict equality check, i.e. the materials must have identical + /// properties. + /// + /// @param lhs is the left hand side material + /// @param rhs is the right hand side material + /// /// @return true if the materials are equal friend constexpr bool operator==(const Material& lhs, const Material& rhs) { return (lhs.m_x0 == rhs.m_x0) && (lhs.m_l0 == rhs.m_l0) && diff --git a/Core/include/Acts/Material/MaterialSlab.hpp b/Core/include/Acts/Material/MaterialSlab.hpp index e94e063e9cc..63fd7378e8b 100644 --- a/Core/include/Acts/Material/MaterialSlab.hpp +++ b/Core/include/Acts/Material/MaterialSlab.hpp @@ -79,10 +79,14 @@ class MaterialSlab { float m_thicknessInX0 = 0.0f; float m_thicknessInL0 = 0.0f; - /// Check if two materials are exactly equal. - /// @note This is a strict equality check, i.e. the materials must - /// have identical properties. - /// @param other is the material to compare to + /// @brief Check if two materials are exactly equal. + /// + /// This is a strict equality check, i.e. the materials must have identical + /// properties. + /// + /// @param lhs is the left hand side material + /// @param rhs is the right hand side material + /// /// @return true if the materials are equal friend constexpr bool operator==(const MaterialSlab& lhs, const MaterialSlab& rhs) {