Skip to content

Commit

Permalink
chore!: Clean some Material code (#3897)
Browse files Browse the repository at this point in the history
Accumulated changed from #3862

- use namespaces in source files
- remove operator overloads (breaking)
- move some implementations to source
  • Loading branch information
andiwand authored Nov 25, 2024
1 parent d6b1ee3 commit c5391be
Show file tree
Hide file tree
Showing 18 changed files with 131 additions and 110 deletions.
8 changes: 3 additions & 5 deletions Core/include/Acts/Material/BinnedSurfaceMaterial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
#include "Acts/Material/MaterialSlab.hpp"
#include "Acts/Utilities/BinUtility.hpp"

#include <cstddef>
#include <iosfwd>
#include <vector>

namespace Acts {

Expand Down Expand Up @@ -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;
Expand Down
14 changes: 6 additions & 8 deletions Core/include/Acts/Material/GridSurfaceMaterial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <ostream>
#include <stdexcept>
Expand Down Expand Up @@ -44,7 +42,7 @@ struct GridMaterialAccessor {
///
/// @note this is not particularly fast
template <typename grid_type>
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);
Expand Down Expand Up @@ -74,7 +72,7 @@ struct IndexedMaterialAccessor {
///
/// @param scale the amount of the scaling
template <typename grid_type>
void scale(grid_type& /*grid*/, ActsScalar scale) {
void scale(grid_type& /*grid*/, double scale) {
for (auto& m : material) {
m.scaleThickness(scale);
}
Expand Down Expand Up @@ -118,7 +116,7 @@ struct GloballyIndexedMaterialAccessor {
/// outcome is unpredictable.
///
template <typename grid_type>
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 "
Expand Down Expand Up @@ -197,9 +195,9 @@ class GridSurfaceMaterialT : public ISurfaceMaterial {

/// Scale operator
///
/// @param scale is the scale factor applied
ISurfaceMaterial& operator*=(ActsScalar scale) final {
m_materialAccessor.scale(m_grid, scale);
/// @param factor is the scale factor applied
ISurfaceMaterial& scale(double factor) final {
m_materialAccessor.scale(m_grid, factor);
return (*this);
}

Expand Down
41 changes: 17 additions & 24 deletions Core/include/Acts/Material/HomogeneousSurfaceMaterial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "Acts/Material/ISurfaceMaterial.hpp"
#include "Acts/Material/MaterialSlab.hpp"

#include <cstddef>
#include <iosfwd>

namespace Acts {
Expand Down Expand Up @@ -62,13 +61,8 @@ class HomogeneousSurfaceMaterial : public ISurfaceMaterial {
/// 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;
/// @param factor is the scale factor
HomogeneousSurfaceMaterial& scale(double factor) final;

/// @copydoc ISurfaceMaterial::materialSlab(const Vector2&) const
///
Expand All @@ -94,22 +88,21 @@ class HomogeneousSurfaceMaterial : public ISurfaceMaterial {

private:
/// The five different MaterialSlab
MaterialSlab m_fullMaterial = MaterialSlab();
};

inline const MaterialSlab& HomogeneousSurfaceMaterial::materialSlab(
const Vector2& /*lp*/) const {
return (m_fullMaterial);
}
MaterialSlab 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);
}
/// @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) {
return lhs.m_fullMaterial == rhs.m_fullMaterial;
}
};

} // namespace Acts
31 changes: 15 additions & 16 deletions Core/include/Acts/Material/HomogeneousVolumeMaterial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -64,17 +59,21 @@ class HomogeneousVolumeMaterial : public IVolumeMaterial {
std::ostream& toStream(std::ostream& sl) const final;

private:
Material m_material = Material();
};

inline const Material HomogeneousVolumeMaterial::material(
const Vector3& /*position*/) const {
return (m_material);
}
Material m_material;

inline bool HomogeneousVolumeMaterial::operator==(
const HomogeneousVolumeMaterial& hvm) const {
return (m_material == hvm.m_material);
}
/// @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) {
return lhs.m_material == rhs.m_material;
}
};

} // namespace Acts
17 changes: 8 additions & 9 deletions Core/include/Acts/Material/ISurfaceMaterial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <memory>
#include <sstream>
#include <vector>

namespace Acts {

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -128,9 +125,11 @@ 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,
Expand Down
14 changes: 11 additions & 3 deletions Core/include/Acts/Material/Material.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@

#pragma once

#include "Acts/Definitions/Algebra.hpp"

#include <iosfwd>
#include <limits>
#include <utility>

#include <Eigen/Dense>

namespace Acts {

Expand Down Expand Up @@ -112,6 +111,15 @@ class Material {
float m_z = 0.0f;
float m_molarRho = 0.0f;

/// @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) &&
(lhs.m_ar == rhs.m_ar) && (lhs.m_z == rhs.m_z) &&
Expand Down
9 changes: 9 additions & 0 deletions Core/include/Acts/Material/MaterialSlab.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ class MaterialSlab {
float m_thicknessInX0 = 0.0f;
float m_thicknessInL0 = 0.0f;

/// @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) {
// t/X0 and t/L0 are dependent variables and need not be checked
Expand Down
5 changes: 2 additions & 3 deletions Core/include/Acts/Material/ProtoSurfaceMaterial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "Acts/Material/MaterialSlab.hpp"
#include "Acts/Utilities/BinUtility.hpp"

#include <cstddef>
#include <iosfwd>

namespace Acts {
Expand Down Expand Up @@ -67,9 +66,9 @@ class ProtoSurfaceMaterialT : public ISurfaceMaterial {
ProtoSurfaceMaterialT<BinningType>& operator=(
ProtoSurfaceMaterialT<BinningType>&& smproxy) = default;

/// Scale operator - dummy implementation
/// Scale operation - dummy implementation
///
ProtoSurfaceMaterialT<BinningType>& operator*=(double /*scale*/) final {
ProtoSurfaceMaterialT<BinningType>& scale(double /*factor*/) final {
return (*this);
}

Expand Down
5 changes: 2 additions & 3 deletions Core/src/Material/BinnedSurfaceMaterial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
31 changes: 22 additions & 9 deletions Core/src/Material/HomogeneousSurfaceMaterial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,33 @@

#include <ostream>

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);
HomogeneousSurfaceMaterial& HomogeneousSurfaceMaterial::scale(double factor) {
m_fullMaterial.scaleThickness(factor);
return *this;
}

const MaterialSlab& HomogeneousSurfaceMaterial::materialSlab(
const Vector2& /*lp*/) const {
return m_fullMaterial;
}

std::ostream& Acts::HomogeneousSurfaceMaterial::toStream(
std::ostream& sl) const {
sl << "Acts::HomogeneousSurfaceMaterial : " << std::endl;
const MaterialSlab& HomogeneousSurfaceMaterial::materialSlab(
const Vector3& /*gp*/) const {
return m_fullMaterial;
}

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
17 changes: 12 additions & 5 deletions Core/src/Material/HomogeneousVolumeMaterial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,20 @@

#include <ostream>

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;
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
Loading

0 comments on commit c5391be

Please sign in to comment.