From f3e39974a03f55f454a2310d83c017834d53e42a Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Sat, 11 Jan 2025 21:52:57 +0100 Subject: [PATCH] fix!: Make BinUtility constructor explicit (#3953) ## 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. --- Core/include/Acts/Utilities/BinUtility.hpp | 4 ++-- Plugins/Detray/src/DetrayMaterialConverter.cpp | 2 +- Plugins/Json/src/MaterialJsonConverter.cpp | 2 +- .../Algorithms/Digitization/ModuleClustersTests.cpp | 9 +++++---- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Core/include/Acts/Utilities/BinUtility.hpp b/Core/include/Acts/Utilities/BinUtility.hpp index a1213ddd8ec..b6038fb5181 100644 --- a/Core/include/Acts/Utilities/BinUtility.hpp +++ b/Core/include/Acts/Utilities/BinUtility.hpp @@ -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); diff --git a/Plugins/Detray/src/DetrayMaterialConverter.cpp b/Plugins/Detray/src/DetrayMaterialConverter.cpp index f6b61236ec5..e4b7c5b6caa 100644 --- a/Plugins/Detray/src/DetrayMaterialConverter.cpp +++ b/Plugins/Detray/src/DetrayMaterialConverter.cpp @@ -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; } diff --git a/Plugins/Json/src/MaterialJsonConverter.cpp b/Plugins/Json/src/MaterialJsonConverter.cpp index ce582850e3c..759e1fd032b 100644 --- a/Plugins/Json/src/MaterialJsonConverter.cpp +++ b/Plugins/Json/src/MaterialJsonConverter.cpp @@ -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; } diff --git a/Tests/UnitTests/Examples/Algorithms/Digitization/ModuleClustersTests.cpp b/Tests/UnitTests/Examples/Algorithms/Digitization/ModuleClustersTests.cpp index a1947cde17e..f2913475b4a 100644 --- a/Tests/UnitTests/Examples/Algorithms/Digitization/ModuleClustersTests.cpp +++ b/Tests/UnitTests/Examples/Algorithms/Digitization/ModuleClustersTests.cpp @@ -8,6 +8,7 @@ #include +#include "Acts/Utilities/BinUtility.hpp" #include "Acts/Utilities/BinningData.hpp" #include "ActsExamples/Digitization/ModuleClusters.hpp" #include "ActsFatras/Digitization/Segmentizer.hpp" @@ -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 boundIndices = {eBoundLoc0, eBoundLoc1}; double nsigma = 1; bool commonCorner = true;