Skip to content

Commit

Permalink
refactor: remove operator== from digitization config objects (#1875)
Browse files Browse the repository at this point in the history 
There is an operator== defined for `GeometricConfig` and `SmearingConfig`, however, this operator does not really test equality, since it cannot check the involved `std::function` objects. This PR removes this operator, since `==` has a clear defined semantics and should not be used if equality is not checked actually.
  • Loading branch information
@benjaminhuth
benjaminhuth committed Feb 23, 2023
1 parent 78be399 commit 0ce93a5
Show file tree
Hide file tree
Showing 5 changed files with 252 additions and 246 deletions.
1 change: 1 addition & 0 deletions Examples/Algorithms/Digitization/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ add_library(
src/DigitizationConfig.cpp
src/MeasurementCreation.cpp
src/PlanarSteppingAlgorithm.cpp
src/DigitizationConfigurator.cpp
src/ModuleClusters.cpp)
target_include_directories(
ActsExamplesDigitization
Expand Down
18 changes: 0 additions & 18 deletions Examples/Algorithms/Digitization/include/ActsExamples/Digitization/DigitizationConfig.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,6 @@ struct GeometricConfig {
RandomEngine & /*unused*/) const {
return Acts::Vector3(0., 0., 0.);
};

/// Equality operator for basic parameters
/// check if the geometry config can be reused from
/// @param other, @return a boolean to indicate this
bool operator==(const GeometricConfig &other) const {
return (indices == other.indices and segmentation == other.segmentation and
thickness == other.thickness and threshold == other.threshold and
digital == other.digital);
}
};

/// Configuration struct for the Digitization algorithm
Expand All @@ -103,15 +94,6 @@ struct GeometricConfig {
struct DigiComponentsConfig {
GeometricConfig geometricDigiConfig;
SmearingConfig smearingDigiConfig = {};

/// Equality operator to check if a digitization configuration
/// can be reused from @param other
///
/// @return a boolean flag indicating equality
bool operator==(const DigiComponentsConfig &other) const {
return (geometricDigiConfig == other.geometricDigiConfig and
smearingDigiConfig == other.smearingDigiConfig);
}
};

class DigitizationConfig {
Expand Down
221 changes: 1 addition & 220 deletions ...es/Algorithms/Digitization/include/ActsExamples/Digitization/DigitizationConfigurator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,225 +58,6 @@ struct DigitizationConfigurator {
/// @param surface is the surfaces that is visited
///
/// it adds an appropriate entry into the digitisation configuration
void operator()(const Acts::Surface* surface) {
if (surface->associatedDetectorElement() != nullptr) {
Acts::GeometryIdentifier geoId = surface->geometryId();
auto dInputConfig = inputDigiComponents.find((geoId));
if (dInputConfig != inputDigiComponents.end()) {
// The output config, copy over the smearing part
DigiComponentsConfig dOutputConfig;
dOutputConfig.smearingDigiConfig = dInputConfig->smearingDigiConfig;

if (not dInputConfig->geometricDigiConfig.indices.empty()) {
// Copy over what can be done
dOutputConfig.geometricDigiConfig.indices =
dInputConfig->geometricDigiConfig.indices;
dOutputConfig.geometricDigiConfig.thickness =
dInputConfig->geometricDigiConfig.thickness;
dOutputConfig.geometricDigiConfig.chargeSmearer =
dInputConfig->geometricDigiConfig.chargeSmearer;
dOutputConfig.geometricDigiConfig.digital =
dInputConfig->geometricDigiConfig.digital;

const Acts::SurfaceBounds& sBounds = surface->bounds();
auto boundValues = sBounds.values();

const auto& inputSegmentation =
dInputConfig->geometricDigiConfig.segmentation;
Acts::BinUtility outputSegmentation;

switch (sBounds.type()) {
// The module is a rectangle module
case Acts::SurfaceBounds::eRectangle: {
if (inputSegmentation.binningData()[0].binvalue == Acts::binX) {
Acts::ActsScalar minX =
boundValues[Acts::RectangleBounds::eMinX];
Acts::ActsScalar maxX =
boundValues[Acts::RectangleBounds::eMaxX];
unsigned int nBins = static_cast<unsigned int>(std::round(
(maxX - minX) / inputSegmentation.binningData()[0].step));
outputSegmentation +=
Acts::BinUtility(nBins, minX, maxX, Acts::open, Acts::binX);
}
if (inputSegmentation.binningData()[0].binvalue == Acts::binY or
inputSegmentation.dimensions() == 2) {
unsigned int accessBin =
inputSegmentation.dimensions() == 2 ? 1 : 0;
Acts::ActsScalar minY =
boundValues[Acts::RectangleBounds::eMinY];
Acts::ActsScalar maxY =
boundValues[Acts::RectangleBounds::eMaxY];
unsigned int nBins = static_cast<unsigned int>(std::round(
(maxY - minY) /
inputSegmentation.binningData()[accessBin].step));
outputSegmentation +=
Acts::BinUtility(nBins, minY, maxY, Acts::open, Acts::binY);
}
} break;

// The module is a trapezoid module
case Acts::SurfaceBounds::eTrapezoid: {
if (inputSegmentation.binningData()[0].binvalue == Acts::binX) {
Acts::ActsScalar maxX = std::max(
boundValues[Acts::TrapezoidBounds::eHalfLengthXnegY],
boundValues[Acts::TrapezoidBounds::eHalfLengthXposY]);
unsigned int nBins = static_cast<unsigned int>(std::round(
2 * maxX / inputSegmentation.binningData()[0].step));
outputSegmentation += Acts::BinUtility(nBins, -maxX, maxX,
Acts::open, Acts::binX);
}
if (inputSegmentation.binningData()[0].binvalue == Acts::binY or
inputSegmentation.dimensions() == 2) {
unsigned int accessBin =
inputSegmentation.dimensions() == 2 ? 1 : 0;
Acts::ActsScalar maxY =
boundValues[Acts::TrapezoidBounds::eHalfLengthY];
unsigned int nBins = static_cast<unsigned int>(std::round(
(2 * maxY) /
inputSegmentation.binningData()[accessBin].step));
outputSegmentation += Acts::BinUtility(nBins, -maxY, maxY,
Acts::open, Acts::binY);
}
} break;

// The module is an annulus module
case Acts::SurfaceBounds::eAnnulus: {
if (inputSegmentation.binningData()[0].binvalue == Acts::binR) {
Acts::ActsScalar minR = boundValues[Acts::AnnulusBounds::eMinR];
Acts::ActsScalar maxR = boundValues[Acts::AnnulusBounds::eMaxR];
unsigned int nBins = static_cast<unsigned int>(std::round(
(maxR - minR) / inputSegmentation.binningData()[0].step));
outputSegmentation +=
Acts::BinUtility(nBins, minR, maxR, Acts::open, Acts::binR);
}
if (inputSegmentation.binningData()[0].binvalue == Acts::binPhi or
inputSegmentation.dimensions() == 2) {
unsigned int accessBin =
inputSegmentation.dimensions() == 2 ? 1 : 0;
Acts::ActsScalar averagePhi =
boundValues[Acts::AnnulusBounds::eAveragePhi];
Acts::ActsScalar minPhi =
averagePhi - boundValues[Acts::AnnulusBounds::eMinPhiRel];
Acts::ActsScalar maxPhi =
averagePhi + boundValues[Acts::AnnulusBounds::eMaxPhiRel];
unsigned int nBins = static_cast<unsigned int>(std::round(
(maxPhi - minPhi) /
inputSegmentation.binningData()[accessBin].step));
outputSegmentation += Acts::BinUtility(
nBins, minPhi, maxPhi, Acts::open, Acts::binPhi);
}

} break;

// The module is a Disc Trapezoid
case Acts::SurfaceBounds::eDiscTrapezoid: {
Acts::ActsScalar minR =
boundValues[Acts::DiscTrapezoidBounds::eMinR];
Acts::ActsScalar maxR =
boundValues[Acts::DiscTrapezoidBounds::eMaxR];

if (inputSegmentation.binningData()[0].binvalue == Acts::binR) {
unsigned int nBins = static_cast<unsigned int>(std::round(
(maxR - minR) / inputSegmentation.binningData()[0].step));
outputSegmentation +=
Acts::BinUtility(nBins, minR, maxR, Acts::open, Acts::binR);
}
if (inputSegmentation.binningData()[0].binvalue == Acts::binPhi or
inputSegmentation.dimensions() == 2) {
unsigned int accessBin =
inputSegmentation.dimensions() == 2 ? 1 : 0;
Acts::ActsScalar hxMinR =
boundValues[Acts::DiscTrapezoidBounds::eHalfLengthXminR];
Acts::ActsScalar hxMaxR =
boundValues[Acts::DiscTrapezoidBounds::eHalfLengthXmaxR];

Acts::ActsScalar averagePhi =
boundValues[Acts::DiscTrapezoidBounds::eAveragePhi];
Acts::ActsScalar alphaMinR = std::atan2(minR, hxMinR);
Acts::ActsScalar alphaMaxR = std::atan2(maxR, hxMaxR);
Acts::ActsScalar alpha = std::max(alphaMinR, alphaMaxR);
unsigned int nBins = static_cast<unsigned int>(std::round(
2 * alpha /
inputSegmentation.binningData()[accessBin].step));
outputSegmentation += Acts::BinUtility(
nBins, averagePhi - alpha, averagePhi + alpha, Acts::open,
Acts::binPhi);
}

} break;

case Acts::SurfaceBounds::eDisc: {
if (inputSegmentation.binningData()[0].binvalue == Acts::binR) {
Acts::ActsScalar minR = boundValues[Acts::RadialBounds::eMinR];
Acts::ActsScalar maxR = boundValues[Acts::RadialBounds::eMaxR];
unsigned int nBins = static_cast<unsigned int>(std::round(
(maxR - minR) / inputSegmentation.binningData()[0].step));
outputSegmentation +=
Acts::BinUtility(nBins, minR, maxR, Acts::open, Acts::binR);
}
if (inputSegmentation.binningData()[0].binvalue == Acts::binPhi or
inputSegmentation.dimensions() == 2) {
unsigned int accessBin =
inputSegmentation.dimensions() == 2 ? 1 : 0;

Acts::ActsScalar averagePhi =
boundValues[Acts::RadialBounds::eAveragePhi];
Acts::ActsScalar halfPhiSector =
boundValues[Acts::RadialBounds::eHalfPhiSector];
Acts::ActsScalar minPhi = averagePhi - halfPhiSector;
Acts::ActsScalar maxPhi = averagePhi + halfPhiSector;

unsigned int nBins = static_cast<unsigned int>(std::round(
(maxPhi - minPhi) /
inputSegmentation.binningData()[accessBin].step));
outputSegmentation += Acts::BinUtility(
nBins, minPhi, maxPhi, Acts::open, Acts::binPhi);
}

} break;

default:
break;
}
// Set the adapted segmentation class
dOutputConfig.geometricDigiConfig.segmentation = outputSegmentation;
}

// Compactify the output map where possible
if (compactify) {
// Check for a representing volume configuration, insert if not
// present
Acts::GeometryIdentifier volGeoId =
Acts::GeometryIdentifier().setVolume(geoId.volume());

auto volRep = volumeLayerComponents.find(volGeoId);
if (volRep != volumeLayerComponents.end() and
dOutputConfig == volRep->second) {
// return if the volume representation already covers this one
return;
} else {
volumeLayerComponents[volGeoId] = dOutputConfig;
outputDigiComponents.push_back({volGeoId, dOutputConfig});
}

// Check for a representing layer configuration, insert if not present
Acts::GeometryIdentifier volLayGeoId =
Acts::GeometryIdentifier(volGeoId).setLayer(geoId.layer());
auto volLayRep = volumeLayerComponents.find(volLayGeoId);

if (volLayRep != volumeLayerComponents.end() and
dOutputConfig == volLayRep->second) {
return;
} else {
volumeLayerComponents[volLayGeoId] = dOutputConfig;
outputDigiComponents.push_back({volLayGeoId, dOutputConfig});
}
}

// Insert into the output list
outputDigiComponents.push_back({geoId, dOutputConfig});
}
}
}
void operator()(const Acts::Surface* surface);
};
} // namespace ActsExamples
8 changes: 0 additions & 8 deletions Examples/Algorithms/Digitization/include/ActsExamples/Digitization/SmearingConfig.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@ struct ParameterSmearingConfig {
Acts::BoundIndices index = Acts::eBoundSize;
/// The smearing function for this parameter.
ActsFatras::SingleParameterSmearFunction<RandomEngine> smearFunction;

/// Check if the smearing configuration is the same
/// @param other is the one to be checked against
///
/// @return boolean to indicate equality
bool operator==(const ParameterSmearingConfig& other) const {
return (index == other.index);
}
};

// The configured indices must be unique, i.e. each one can only appear once
Expand Down

0 comments on commit 0ce93a5

Please sign in to comment.