Skip to content

Commit

Permalink
fix!: Make BinUtility constructor explicit (#3953)
Browse files Browse the repository at this point in the history
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Enhanced JSON serialization and deserialization for material types,
including robust handling for binned materials.
- Improved error handling for unsupported binning configurations during
JSON conversion.
	- Updated constructor for `BinUtility` to prevent implicit conversions.

- **Bug Fixes**
- Adjusted grid index type assignment based on binning values to prevent
runtime errors.
	- Improved error handling and logging in material conversion processes.

- **Tests**
- Updated test cases for `BinUtility` initialization to improve clarity
without altering functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
paulgessinger authored Jan 11, 2025
1 parent d6eb834 commit f3e3997
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 8 deletions.
4 changes: 2 additions & 2 deletions Core/include/Acts/Utilities/BinUtility.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ class BinUtility {
///
/// @param bData is the provided binning data
/// @param tForm is the (optional) transform
BinUtility(const BinningData& bData,
const Transform3& tForm = Transform3::Identity())
explicit BinUtility(const BinningData& bData,
const Transform3& tForm = Transform3::Identity())
: m_binningData(), m_transform(tForm), m_itransform(tForm.inverse()) {
m_binningData.reserve(3);
m_binningData.push_back(bData);
Expand Down
2 changes: 1 addition & 1 deletion Plugins/Detray/src/DetrayMaterialConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ Acts::DetrayMaterialConverter::convertGridSurfaceMaterial(
bUtility.binningData()[0u].binvalue == BinningValue::binZ &&
bUtility.binningData()[1u].binvalue == BinningValue::binPhi) {
BinUtility nbUtility(bUtility.binningData()[1u]);
nbUtility += bUtility.binningData()[0u];
nbUtility += BinUtility{bUtility.binningData()[0u]};
bUtility = std::move(nbUtility);
swapped = true;
}
Expand Down
2 changes: 1 addition & 1 deletion Plugins/Json/src/MaterialJsonConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ nlohmann::json Acts::MaterialJsonConverter::toJsonDetray(
bUtility.binningData()[0u].binvalue == BinningValue::binZ &&
bUtility.binningData()[1u].binvalue == BinningValue::binPhi) {
BinUtility nbUtility(bUtility.binningData()[1u]);
nbUtility += bUtility.binningData()[0u];
nbUtility += BinUtility{bUtility.binningData()[0u]};
bUtility = std::move(nbUtility);
swapped = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <boost/test/unit_test.hpp>

#include "Acts/Utilities/BinUtility.hpp"
#include "Acts/Utilities/BinningData.hpp"
#include "ActsExamples/Digitization/ModuleClusters.hpp"
#include "ActsFatras/Digitization/Segmentizer.hpp"
Expand Down Expand Up @@ -46,10 +47,10 @@ DigitizedParameters makeDigitizationParameters(const Vector2 &position,
auto testDigitizedParametersWithTwoClusters(bool merge, const Vector2 &firstHit,
const Vector2 &secondHit) {
BinUtility binUtility;
binUtility +=
BinningData(BinningOption::open, BinningValue::binX, 20, -10.0f, 10.0f);
binUtility +=
BinningData(BinningOption::open, BinningValue::binY, 20, -10.0f, 10.0f);
binUtility += BinUtility{
BinningData(BinningOption::open, BinningValue::binX, 20, -10.0f, 10.0f)};
binUtility += BinUtility{
BinningData(BinningOption::open, BinningValue::binY, 20, -10.0f, 10.0f)};
std::vector<Acts::BoundIndices> boundIndices = {eBoundLoc0, eBoundLoc1};
double nsigma = 1;
bool commonCorner = true;
Expand Down

0 comments on commit f3e3997

Please sign in to comment.