From 6e3a9a683e66cb28169f15f8d8f6c8390e01651d Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 22 Nov 2024 12:19:39 +0100 Subject: [PATCH 1/2] refactor: Avoid `new`/`delete` in a number of places. (#3828) --- .../Acts/Geometry/SurfaceArrayCreator.hpp | 16 +++--- Core/include/Acts/Seeding/GbtsDataStorage.hpp | 55 +++++++------------ Core/include/Acts/Seeding/GbtsGeometry.hpp | 38 ++++--------- Core/include/Acts/Seeding/SeedFinderGbts.ipp | 28 ++++------ .../Acts/TrackFinding/GbtsConnector.hpp | 9 +-- Core/src/Geometry/LayerCreator.cpp | 3 +- Core/src/TrackFinding/GbtsConnector.cpp | 19 ++----- .../GenericDetector/ProtoLayerCreatorT.hpp | 15 +++-- .../MagneticField/src/FieldMapRootIo.cpp | 6 +- .../Io/Root/RootAthenaNTupleReader.hpp | 4 +- .../Io/Root/RootMaterialTrackReader.hpp | 2 +- .../Io/Root/RootParticleReader.hpp | 2 +- .../ActsExamples/Io/Root/RootSimHitReader.hpp | 4 +- .../Io/Root/RootTrackSummaryReader.hpp | 2 +- .../ActsExamples/Io/Root/RootVertexReader.hpp | 2 +- .../Io/Root/src/RootAthenaNTupleReader.cpp | 4 +- Examples/Io/Root/src/RootBFieldWriter.cpp | 7 +-- .../Io/Root/src/RootMaterialTrackReader.cpp | 4 +- Examples/Io/Root/src/RootParticleReader.cpp | 2 +- Examples/Io/Root/src/RootSimHitReader.cpp | 4 +- .../Io/Root/src/RootTrackSummaryReader.cpp | 2 +- Examples/Io/Root/src/RootVertexReader.cpp | 2 +- .../NuclearInteractionParametrisation.cpp | 13 +++-- 23 files changed, 100 insertions(+), 143 deletions(-) diff --git a/Core/include/Acts/Geometry/SurfaceArrayCreator.hpp b/Core/include/Acts/Geometry/SurfaceArrayCreator.hpp index 584e8ee6992..9906fda7939 100644 --- a/Core/include/Acts/Geometry/SurfaceArrayCreator.hpp +++ b/Core/include/Acts/Geometry/SurfaceArrayCreator.hpp @@ -381,8 +381,8 @@ class SurfaceArrayCreator { Axis axisB(pAxisB.min, pAxisB.max, pAxisB.nBins); using SGL = SurfaceArray::SurfaceGridLookup; - ptr = std::unique_ptr(static_cast( - new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue}))); + ptr = std::make_unique( + globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue}); } else if (pAxisA.bType == equidistant && pAxisB.bType == arbitrary) { @@ -390,8 +390,8 @@ class SurfaceArrayCreator { Axis axisB(pAxisB.binEdges); using SGL = SurfaceArray::SurfaceGridLookup; - ptr = std::unique_ptr(static_cast( - new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue}))); + ptr = std::make_unique( + globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue}); } else if (pAxisA.bType == arbitrary && pAxisB.bType == equidistant) { @@ -399,8 +399,8 @@ class SurfaceArrayCreator { Axis axisB(pAxisB.min, pAxisB.max, pAxisB.nBins); using SGL = SurfaceArray::SurfaceGridLookup; - ptr = std::unique_ptr(static_cast( - new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue}))); + ptr = std::make_unique( + globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue}); } else /*if (pAxisA.bType == arbitrary && pAxisB.bType == arbitrary)*/ { @@ -408,8 +408,8 @@ class SurfaceArrayCreator { Axis axisB(pAxisB.binEdges); using SGL = SurfaceArray::SurfaceGridLookup; - ptr = std::unique_ptr(static_cast( - new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue}))); + ptr = std::make_unique( + globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue}); } // clang-format on diff --git a/Core/include/Acts/Seeding/GbtsDataStorage.hpp b/Core/include/Acts/Seeding/GbtsDataStorage.hpp index c28b0cc2413..8c7ba861bcd 100644 --- a/Core/include/Acts/Seeding/GbtsDataStorage.hpp +++ b/Core/include/Acts/Seeding/GbtsDataStorage.hpp @@ -13,6 +13,7 @@ #include "Acts/Seeding/GbtsGeometry.hpp" #include +#include #include #include #include @@ -47,13 +48,6 @@ struct GbtsSP { template class GbtsNode { public: - struct CompareByPhi { - bool operator()(const GbtsNode *n1, - const GbtsNode *n2) { - return (n1->m_spGbts.phi() < n2->m_spGbts.phi()); - } - }; - GbtsNode(const GbtsSP &spGbts, float minT = -100.0, float maxT = 100.0) : m_spGbts(spGbts), m_minCutOnTau(minT), m_maxCutOnTau(maxT) {} @@ -97,27 +91,20 @@ class GbtsEtaBin { public: GbtsEtaBin() { m_vn.clear(); } - ~GbtsEtaBin() { - for (typename std::vector *>::iterator it = - m_vn.begin(); - it != m_vn.end(); ++it) { - delete (*it); - } - } - void sortByPhi() { - std::ranges::sort(m_vn, - typename Acts::GbtsNode::CompareByPhi()); + std::ranges::sort(m_vn, [](const auto &n1, const auto &n2) { + return (n1->m_spGbts.phi() < n2->m_spGbts.phi()); + }); } bool empty() const { return m_vn.empty(); } void generatePhiIndexing(float dphi) { for (unsigned int nIdx = 0; nIdx < m_vn.size(); nIdx++) { - GbtsNode *pN = m_vn.at(nIdx); + GbtsNode &pN = *m_vn.at(nIdx); // float phi = pN->m_sp.phi(); // float phi = (std::atan(pN->m_sp.x() / pN->m_sp.y())); - float phi = pN->m_spGbts.phi(); + float phi = pN.m_spGbts.phi(); if (phi <= std::numbers::pi_v - dphi) { continue; } @@ -127,14 +114,14 @@ class GbtsEtaBin { } for (unsigned int nIdx = 0; nIdx < m_vn.size(); nIdx++) { - GbtsNode *pN = m_vn.at(nIdx); - float phi = pN->m_spGbts.phi(); + GbtsNode &pN = *m_vn.at(nIdx); + float phi = pN.m_spGbts.phi(); m_vPhiNodes.push_back(std::pair(phi, nIdx)); } for (unsigned int nIdx = 0; nIdx < m_vn.size(); nIdx++) { - GbtsNode *pN = m_vn.at(nIdx); - float phi = pN->m_spGbts.phi(); + GbtsNode &pN = *m_vn.at(nIdx); + float phi = pN.m_spGbts.phi(); if (phi >= -std::numbers::pi_v + dphi) { break; } @@ -143,9 +130,7 @@ class GbtsEtaBin { } } - std::vector *> m_vn; - // TODO change to - // std::vector>> m_vn; + std::vector>> m_vn; std::vector> m_vPhiNodes; }; @@ -186,8 +171,9 @@ class GbtsDataStorage { 1.6 + 0.15 / (cluster_width + 0.2) + 6.1 * (cluster_width - 0.2); } - m_etaBins.at(binIndex).m_vn.push_back(new GbtsNode( - sp, min_tau, max_tau)); // adding ftf member to nodes + m_etaBins.at(binIndex).m_vn.push_back( + std::make_unique>( + sp, min_tau, max_tau)); // adding ftf member to nodes } else { if (useClusterWidth) { float cluster_width = 1; // temporary while cluster width not available @@ -195,7 +181,8 @@ class GbtsDataStorage { return -3; } } - m_etaBins.at(binIndex).m_vn.push_back(new GbtsNode(sp)); + m_etaBins.at(binIndex).m_vn.push_back( + std::make_unique>(sp)); } return 0; @@ -218,16 +205,14 @@ class GbtsDataStorage { vn.clear(); vn.reserve(numberOfNodes()); for (const auto &b : m_etaBins) { - for (typename std::vector *>::const_iterator nIt = - b.m_vn.begin(); - nIt != b.m_vn.end(); ++nIt) { - if ((*nIt)->m_in.empty()) { + for (const auto &n : b.m_vn) { + if (n->m_in.empty()) { continue; } - if ((*nIt)->m_out.empty()) { + if (n->m_out.empty()) { continue; } - vn.push_back(*nIt); + vn.push_back(n.get()); } } } diff --git a/Core/include/Acts/Seeding/GbtsGeometry.hpp b/Core/include/Acts/Seeding/GbtsGeometry.hpp index 54931b200b8..cc1af647d28 100644 --- a/Core/include/Acts/Seeding/GbtsGeometry.hpp +++ b/Core/include/Acts/Seeding/GbtsGeometry.hpp @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -277,15 +278,10 @@ class GbtsGeometry { // calculating bin tables in the connector... - for (std::map>::const_iterator it = - m_connector->m_connMap.begin(); - it != m_connector->m_connMap.end(); ++it) { - const std::vector &vConn = (*it).second; - - for (std::vector::const_iterator cIt = vConn.begin(); - cIt != vConn.end(); ++cIt) { - unsigned int src = (*cIt)->m_src; // n2 : the new connectors - unsigned int dst = (*cIt)->m_dst; // n1 + for (auto &[_, vConn] : m_connector->m_connMap) { + for (auto &c : vConn) { + unsigned int src = c->m_src; // n2 : the new connectors + unsigned int dst = c->m_dst; // n1 const GbtsLayer *pL1 = getGbtsLayerByKey(dst); const GbtsLayer *pL2 = getGbtsLayerByKey(src); @@ -301,7 +297,7 @@ class GbtsGeometry { int nSrcBins = pL2->m_bins.size(); int nDstBins = pL1->m_bins.size(); - (*cIt)->m_binTable.resize(nSrcBins * nDstBins, 0); + c->m_binTable.resize(nSrcBins * nDstBins, 0); for (int b1 = 0; b1 < nDstBins; b1++) { // loop over bins in Layer 1 for (int b2 = 0; b2 < nSrcBins; b2++) { // loop over bins in Layer 2 @@ -309,7 +305,7 @@ class GbtsGeometry { continue; } int address = b1 + b2 * nDstBins; - (*cIt)->m_binTable.at(address) = 1; + c->m_binTable.at(address) = 1; } } } @@ -322,17 +318,6 @@ class GbtsGeometry { GbtsGeometry(const GbtsGeometry &) = delete; GbtsGeometry &operator=(const GbtsGeometry &) = delete; - ~GbtsGeometry() { - for (typename std::vector *>::iterator it = - m_layArray.begin(); - it != m_layArray.end(); ++it) { - delete (*it); - } - - m_layMap.clear(); - m_layArray.clear(); - } - const GbtsLayer *getGbtsLayerByKey(unsigned int key) const { typename std::map *>::const_iterator it = m_layMap.find(key); @@ -344,7 +329,7 @@ class GbtsGeometry { } const GbtsLayer *getGbtsLayerByIndex(int idx) const { - return m_layArray.at(idx); + return m_layArray.at(idx).get(); } int num_bins() const { return m_nEtaBins; } @@ -357,18 +342,19 @@ class GbtsGeometry { unsigned int layerKey = l.m_subdet; // this should be combined ID float ew = m_etaBinWidth; - GbtsLayer *pHL = new GbtsLayer(l, ew, bin0); + auto upHL = std::make_unique>(l, ew, bin0); + auto *pHL = upHL.get(); m_layMap.insert( std::pair *>(layerKey, pHL)); - m_layArray.push_back(pHL); + m_layArray.push_back(std::move(upHL)); return pHL; } float m_etaBinWidth{}; std::map *> m_layMap; - std::vector *> m_layArray; + std::vector>> m_layArray; int m_nEtaBins{0}; diff --git a/Core/include/Acts/Seeding/SeedFinderGbts.ipp b/Core/include/Acts/Seeding/SeedFinderGbts.ipp index ed2fe157acf..fb2722db616 100644 --- a/Core/include/Acts/Seeding/SeedFinderGbts.ipp +++ b/Core/include/Acts/Seeding/SeedFinderGbts.ipp @@ -160,12 +160,7 @@ void SeedFinderGbts::runGbts_TrackFinder( } unsigned int first_it = 0; - for (typename std::vector< - GbtsNode*>::const_iterator n1It = - B1.m_vn.begin(); - n1It != B1.m_vn.end(); ++n1It) { // loop over nodes in Layer 1 - - GbtsNode* n1 = (*n1It); + for (const auto& n1 : B1.m_vn) { // loop over nodes in Layer 1 if (n1->m_in.size() >= MAX_SEG_PER_NODE) { continue; @@ -195,7 +190,7 @@ void SeedFinderGbts::runGbts_TrackFinder( } GbtsNode* n2 = - B2.m_vn.at(B2.m_vPhiNodes.at(n2PhiIdx).second); + B2.m_vn.at(B2.m_vPhiNodes.at(n2PhiIdx).second).get(); if (n2->m_out.size() >= MAX_SEG_PER_NODE) { continue; @@ -304,8 +299,8 @@ void SeedFinderGbts::runGbts_TrackFinder( float dPhi1 = std::asin(curv * r1); if (nEdges < m_config.MaxEdges) { - edgeStorage.emplace_back(n1, n2, exp_eta, curv, phi1 + dPhi1, - phi2 + dPhi2); + edgeStorage.emplace_back(n1.get(), n2, exp_eta, curv, + phi1 + dPhi1, phi2 + dPhi2); n1->addIn(nEdges); n2->addOut(nEdges); @@ -332,21 +327,20 @@ void SeedFinderGbts::runGbts_TrackFinder( int nNodes = vNodes.size(); for (int nodeIdx = 0; nodeIdx < nNodes; nodeIdx++) { - const GbtsNode* pN = vNodes.at(nodeIdx); + const GbtsNode& pN = *vNodes.at(nodeIdx); std::vector> in_sort, out_sort; - in_sort.resize(pN->m_in.size()); - out_sort.resize(pN->m_out.size()); + in_sort.resize(pN.m_in.size()); + out_sort.resize(pN.m_out.size()); - for (int inIdx = 0; inIdx < static_cast(pN->m_in.size()); inIdx++) { - int inEdgeIdx = pN->m_in.at(inIdx); + for (int inIdx = 0; inIdx < static_cast(pN.m_in.size()); inIdx++) { + int inEdgeIdx = pN.m_in.at(inIdx); Acts::GbtsEdge* pS = &(edgeStorage.at(inEdgeIdx)); in_sort[inIdx].second = inEdgeIdx; in_sort[inIdx].first = pS->m_p[0]; } - for (int outIdx = 0; outIdx < static_cast(pN->m_out.size()); - outIdx++) { - int outEdgeIdx = pN->m_out.at(outIdx); + for (int outIdx = 0; outIdx < static_cast(pN.m_out.size()); outIdx++) { + int outEdgeIdx = pN.m_out.at(outIdx); Acts::GbtsEdge* pS = &(edgeStorage.at(outEdgeIdx)); out_sort[outIdx].second = outEdgeIdx; out_sort[outIdx].first = pS->m_p[0]; diff --git a/Core/include/Acts/TrackFinding/GbtsConnector.hpp b/Core/include/Acts/TrackFinding/GbtsConnector.hpp index f4cddd2527b..d4724929ac2 100644 --- a/Core/include/Acts/TrackFinding/GbtsConnector.hpp +++ b/Core/include/Acts/TrackFinding/GbtsConnector.hpp @@ -11,8 +11,8 @@ // TODO: update to C++17 style // Consider to moving to detail subdirectory #include -#include #include +#include #include namespace Acts { @@ -39,15 +39,10 @@ class GbtsConnector { GbtsConnector(std::ifstream &inFile); - ~GbtsConnector(); - float m_etaBin{}; std::map> m_layerGroups; - std::map> m_connMap; - // TODO: change to std::map > - // m_connMap; or std::map> > m_connMap; + std::map>> m_connMap; }; } // namespace Acts diff --git a/Core/src/Geometry/LayerCreator.cpp b/Core/src/Geometry/LayerCreator.cpp index 632274c671f..e24ff8e0661 100644 --- a/Core/src/Geometry/LayerCreator.cpp +++ b/Core/src/Geometry/LayerCreator.cpp @@ -407,8 +407,7 @@ Acts::MutableLayerPtr Acts::LayerCreator::planeLayer( } // create the layer and push it back - std::shared_ptr pBounds( - new RectangleBounds(layerHalf1, layerHalf2)); + auto pBounds = std::make_shared(layerHalf1, layerHalf2); // create the layer MutableLayerPtr pLayer = diff --git a/Core/src/TrackFinding/GbtsConnector.cpp b/Core/src/TrackFinding/GbtsConnector.cpp index d947894871c..69e0f7919e5 100644 --- a/Core/src/TrackFinding/GbtsConnector.cpp +++ b/Core/src/TrackFinding/GbtsConnector.cpp @@ -9,7 +9,6 @@ // TODO: update to C++17 style #include "Acts/TrackFinding/GbtsConnector.hpp" -#include #include #include #include @@ -34,7 +33,7 @@ GbtsConnector::GbtsConnector(std::ifstream &inFile) { inFile >> lIdx >> stage >> src >> dst >> height >> width >> nEntries; - GbtsConnection *pC = new GbtsConnection(src, dst); + auto pC = std::make_unique(src, dst); int dummy{}; @@ -47,19 +46,17 @@ GbtsConnector::GbtsConnector(std::ifstream &inFile) { int vol_id = src / 1000; if (vol_id == 13 || vol_id == 12 || vol_id == 14) { - delete pC; continue; } vol_id = dst / 1000; if (vol_id == 13 || vol_id == 12 || vol_id == 14) { - delete pC; continue; } auto &connections = m_connMap[stage]; - connections.push_back(pC); + connections.push_back(std::move(pC)); } // re-arrange the connection stages @@ -69,7 +66,9 @@ GbtsConnector::GbtsConnector(std::ifstream &inFile) { std::map> newConnMap; for (const auto &[_, value] : m_connMap) { - std::ranges::copy(value, std::back_inserter(lConns)); + for (const auto &conn : value) { + lConns.push_back(conn.get()); + } } int stageCounter = 0; @@ -172,12 +171,4 @@ GbtsConnector::GbtsConnector(std::ifstream &inFile) { newConnMap.clear(); } -GbtsConnector::~GbtsConnector() { - m_layerGroups.clear(); - for (const auto &[_, connections] : m_connMap) { - for (auto *conn : connections) { - delete conn; - } - } -} } // namespace Acts diff --git a/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp b/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp index fc4fa4c1459..977c11d9121 100644 --- a/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp +++ b/Examples/Detectors/GenericDetector/include/ActsExamples/GenericDetector/ProtoLayerCreatorT.hpp @@ -218,8 +218,8 @@ ProtoLayerCreatorT::centralProtoLayers( double moduleHalfY = m_cfg.centralModuleHalfY.at(icl); double moduleThickness = m_cfg.centralModuleThickness.at(icl); // create the shared module - std::shared_ptr moduleBounds( - new Acts::RectangleBounds(moduleHalfX, moduleHalfY)); + auto moduleBounds = + std::make_shared(moduleHalfX, moduleHalfY); std::size_t nCentralModules = m_cfg.centralModuleBinningSchema.at(icl).first * m_cfg.centralModuleBinningSchema.at(icl).second; @@ -414,15 +414,14 @@ ProtoLayerCreatorT::createProtoLayers( double moduleHalfY = m_cfg.posnegModuleHalfY.at(ipnl).at(ipnR); // (1) module bounds // create the bounds - Acts::PlanarBounds* pBounds = nullptr; + std::shared_ptr moduleBounds; if (moduleMaxHalfX != 0. && moduleMinHalfX != moduleMaxHalfX) { - pBounds = new Acts::TrapezoidBounds(moduleMinHalfX, moduleMaxHalfX, - moduleHalfY); + moduleBounds = std::make_shared( + moduleMinHalfX, moduleMaxHalfX, moduleHalfY); } else { - pBounds = new Acts::RectangleBounds(moduleMinHalfX, moduleHalfY); + moduleBounds = std::make_shared(moduleMinHalfX, + moduleHalfY); } - // now create the shared bounds from it - std::shared_ptr moduleBounds(pBounds); // (2)) module material // create the Module material from input std::shared_ptr moduleMaterialPtr = diff --git a/Examples/Detectors/MagneticField/src/FieldMapRootIo.cpp b/Examples/Detectors/MagneticField/src/FieldMapRootIo.cpp index bcc93238364..48e2e634d65 100644 --- a/Examples/Detectors/MagneticField/src/FieldMapRootIo.cpp +++ b/Examples/Detectors/MagneticField/src/FieldMapRootIo.cpp @@ -32,7 +32,7 @@ ActsExamples::makeMagneticFieldMapRzFromRoot( // components of magnetic field on grid points std::vector bField; // [1] Read in file and fill values - TFile* inputFile = TFile::Open(fieldMapFile.c_str()); + std::unique_ptr inputFile(TFile::Open(fieldMapFile.c_str())); if (inputFile == nullptr) { throw std::runtime_error("file does not exist"); } @@ -62,7 +62,6 @@ ActsExamples::makeMagneticFieldMapRzFromRoot( zPos.push_back(z); bField.push_back(Acts::Vector2(Br, Bz)); } - delete inputFile; /// [2] use helper function in core return Acts::fieldMapRZ(localToGlobalBin, rPos, zPos, bField, lengthUnit, BFieldUnit, firstQuadrant); @@ -84,7 +83,7 @@ ActsExamples::makeMagneticFieldMapXyzFromRoot( // components of magnetic field on grid points std::vector bField; // [1] Read in file and fill values - TFile* inputFile = TFile::Open(fieldMapFile.c_str()); + std::unique_ptr inputFile(TFile::Open(fieldMapFile.c_str())); if (inputFile == nullptr) { throw std::runtime_error("file does not exist"); } @@ -118,7 +117,6 @@ ActsExamples::makeMagneticFieldMapXyzFromRoot( zPos.push_back(z); bField.push_back(Acts::Vector3(Bx, By, Bz)); } - delete inputFile; return Acts::fieldMapXYZ(localToGlobalBin, xPos, yPos, zPos, bField, lengthUnit, BFieldUnit, firstOctant); diff --git a/Examples/Io/Root/include/ActsExamples/Io/Root/RootAthenaNTupleReader.hpp b/Examples/Io/Root/include/ActsExamples/Io/Root/RootAthenaNTupleReader.hpp index 7f4d6f7bc73..e6bba0782eb 100644 --- a/Examples/Io/Root/include/ActsExamples/Io/Root/RootAthenaNTupleReader.hpp +++ b/Examples/Io/Root/include/ActsExamples/Io/Root/RootAthenaNTupleReader.hpp @@ -178,6 +178,8 @@ class RootAthenaNTupleReader : public ActsExamples::IReader { /// @param config The Configuration struct RootAthenaNTupleReader(const Config &config, Acts::Logging::Level level); + ~RootAthenaNTupleReader() override; + /// Framework name() method std::string name() const final { return "RootAthenaNTupleReader"; } @@ -214,7 +216,7 @@ class RootAthenaNTupleReader : public ActsExamples::IReader { std::size_t m_events = 0; /// The input tree name - TChain *m_inputChain = nullptr; + std::unique_ptr m_inputChain; /// The handle to branches in current event BranchPointerWrapper m_branches; diff --git a/Examples/Io/Root/include/ActsExamples/Io/Root/RootMaterialTrackReader.hpp b/Examples/Io/Root/include/ActsExamples/Io/Root/RootMaterialTrackReader.hpp index 3c3bf2aece3..4c95c7e8d1d 100644 --- a/Examples/Io/Root/include/ActsExamples/Io/Root/RootMaterialTrackReader.hpp +++ b/Examples/Io/Root/include/ActsExamples/Io/Root/RootMaterialTrackReader.hpp @@ -94,7 +94,7 @@ class RootMaterialTrackReader : public IReader { std::size_t m_batchSize = 0; /// The input tree name - TChain* m_inputChain = nullptr; + std::unique_ptr m_inputChain; /// Event identifier. std::uint32_t m_eventId = 0; diff --git a/Examples/Io/Root/include/ActsExamples/Io/Root/RootParticleReader.hpp b/Examples/Io/Root/include/ActsExamples/Io/Root/RootParticleReader.hpp index 0bcd108570c..1e1e4f2adde 100644 --- a/Examples/Io/Root/include/ActsExamples/Io/Root/RootParticleReader.hpp +++ b/Examples/Io/Root/include/ActsExamples/Io/Root/RootParticleReader.hpp @@ -83,7 +83,7 @@ class RootParticleReader : public IReader { std::size_t m_events = 0; /// The input tree name - TChain* m_inputChain = nullptr; + std::unique_ptr m_inputChain; /// Event identifier. std::uint32_t m_eventId = 0; diff --git a/Examples/Io/Root/include/ActsExamples/Io/Root/RootSimHitReader.hpp b/Examples/Io/Root/include/ActsExamples/Io/Root/RootSimHitReader.hpp index bf05678cd8f..7b784b87a58 100644 --- a/Examples/Io/Root/include/ActsExamples/Io/Root/RootSimHitReader.hpp +++ b/Examples/Io/Root/include/ActsExamples/Io/Root/RootSimHitReader.hpp @@ -48,6 +48,8 @@ class RootSimHitReader : public IReader { /// @param config The Configuration struct RootSimHitReader(const Config &config, Acts::Logging::Level level); + ~RootSimHitReader() override; + /// Framework name() method std::string name() const override { return "RootSimHitReader"; } @@ -79,7 +81,7 @@ class RootSimHitReader : public IReader { std::vector> m_eventMap; /// The input tree name - TChain *m_inputChain = nullptr; + std::unique_ptr m_inputChain; /// The keys we have in the ROOT file constexpr static std::array m_floatKeys = { diff --git a/Examples/Io/Root/include/ActsExamples/Io/Root/RootTrackSummaryReader.hpp b/Examples/Io/Root/include/ActsExamples/Io/Root/RootTrackSummaryReader.hpp index 71ff807fb32..1f43918c82b 100644 --- a/Examples/Io/Root/include/ActsExamples/Io/Root/RootTrackSummaryReader.hpp +++ b/Examples/Io/Root/include/ActsExamples/Io/Root/RootTrackSummaryReader.hpp @@ -92,7 +92,7 @@ class RootTrackSummaryReader : public IReader { std::size_t m_events = 0; /// The input tree name - TChain* m_inputChain = nullptr; + std::unique_ptr m_inputChain; /// the event number std::uint32_t m_eventNr{0}; diff --git a/Examples/Io/Root/include/ActsExamples/Io/Root/RootVertexReader.hpp b/Examples/Io/Root/include/ActsExamples/Io/Root/RootVertexReader.hpp index 46afe962bdb..b40c0b9ba6b 100644 --- a/Examples/Io/Root/include/ActsExamples/Io/Root/RootVertexReader.hpp +++ b/Examples/Io/Root/include/ActsExamples/Io/Root/RootVertexReader.hpp @@ -82,7 +82,7 @@ class RootVertexReader : public IReader { std::size_t m_events = 0; /// The input tree name - TChain* m_inputChain = nullptr; + std::unique_ptr m_inputChain; /// Event identifier. std::uint32_t m_eventId = 0; diff --git a/Examples/Io/Root/src/RootAthenaNTupleReader.cpp b/Examples/Io/Root/src/RootAthenaNTupleReader.cpp index ac06349c595..bf61b919c5b 100644 --- a/Examples/Io/Root/src/RootAthenaNTupleReader.cpp +++ b/Examples/Io/Root/src/RootAthenaNTupleReader.cpp @@ -43,7 +43,7 @@ ActsExamples::RootAthenaNTupleReader::RootAthenaNTupleReader( m_outputRecoVtxParameters.initialize(m_cfg.outputRecoVtxParameters); m_outputBeamspotConstraint.initialize(m_cfg.outputBeamspotConstraint); - m_inputChain = new TChain(m_cfg.inputTreeName.c_str()); + m_inputChain = std::make_unique(m_cfg.inputTreeName.c_str()); // unused event identifier std::int32_t eventNumber = 0; @@ -116,6 +116,8 @@ ActsExamples::RootAthenaNTupleReader::RootAthenaNTupleReader( ACTS_DEBUG("The full chain has " << m_events << " entries."); } +ActsExamples::RootAthenaNTupleReader::~RootAthenaNTupleReader() = default; + ActsExamples::ProcessCode ActsExamples::RootAthenaNTupleReader::read( const ActsExamples::AlgorithmContext& context) { ACTS_DEBUG("Trying to read track parameters from ntuple."); diff --git a/Examples/Io/Root/src/RootBFieldWriter.cpp b/Examples/Io/Root/src/RootBFieldWriter.cpp index b8cb8b6c104..894636d61c9 100644 --- a/Examples/Io/Root/src/RootBFieldWriter.cpp +++ b/Examples/Io/Root/src/RootBFieldWriter.cpp @@ -48,13 +48,13 @@ void RootBFieldWriter::run(const Config& config, // Setup ROOT I/O ACTS_INFO("Registering new ROOT output File : " << config.fileName); - TFile* outputFile = - TFile::Open(config.fileName.c_str(), config.fileMode.c_str()); + std::unique_ptr outputFile( + TFile::Open(config.fileName.c_str(), config.fileMode.c_str())); if (outputFile == nullptr) { throw std::ios_base::failure("Could not open '" + config.fileName + "'"); } TTree* outputTree = new TTree(config.treeName.c_str(), - config.treeName.c_str(), 99, outputFile); + config.treeName.c_str(), 99, outputFile.get()); if (outputTree == nullptr) { throw std::bad_alloc(); } @@ -277,6 +277,5 @@ void RootBFieldWriter::run(const Config& config, // Tear down ROOT I/O ACTS_INFO("Closing and Writing ROOT output File : " << config.fileName); outputTree->Write(); - delete outputFile; } } // namespace ActsExamples diff --git a/Examples/Io/Root/src/RootMaterialTrackReader.cpp b/Examples/Io/Root/src/RootMaterialTrackReader.cpp index 70b2ab8d997..b7e5e0ef4f2 100644 --- a/Examples/Io/Root/src/RootMaterialTrackReader.cpp +++ b/Examples/Io/Root/src/RootMaterialTrackReader.cpp @@ -34,7 +34,7 @@ RootMaterialTrackReader::RootMaterialTrackReader(const Config& config, throw std::invalid_argument{"No input files given"}; } - m_inputChain = new TChain(m_cfg.treeName.c_str()); + m_inputChain = std::make_unique(m_cfg.treeName.c_str()); // loop over the input files for (const auto& inputFile : m_cfg.fileList) { @@ -103,8 +103,6 @@ RootMaterialTrackReader::RootMaterialTrackReader(const Config& config, } RootMaterialTrackReader::~RootMaterialTrackReader() { - delete m_inputChain; - delete m_step_x; delete m_step_y; delete m_step_z; diff --git a/Examples/Io/Root/src/RootParticleReader.cpp b/Examples/Io/Root/src/RootParticleReader.cpp index 631fc25ef3e..0eafeaa666b 100644 --- a/Examples/Io/Root/src/RootParticleReader.cpp +++ b/Examples/Io/Root/src/RootParticleReader.cpp @@ -28,7 +28,7 @@ RootParticleReader::RootParticleReader(const RootParticleReader::Config& config, : IReader(), m_cfg(config), m_logger(Acts::getDefaultLogger(name(), level)) { - m_inputChain = new TChain(m_cfg.treeName.c_str()); + m_inputChain = std::make_unique(m_cfg.treeName.c_str()); if (m_cfg.filePath.empty()) { throw std::invalid_argument("Missing input filename"); diff --git a/Examples/Io/Root/src/RootSimHitReader.cpp b/Examples/Io/Root/src/RootSimHitReader.cpp index 4f0d805dc7e..407c70c5dea 100644 --- a/Examples/Io/Root/src/RootSimHitReader.cpp +++ b/Examples/Io/Root/src/RootSimHitReader.cpp @@ -29,7 +29,7 @@ RootSimHitReader::RootSimHitReader(const RootSimHitReader::Config& config, : IReader(), m_cfg(config), m_logger(Acts::getDefaultLogger(name(), level)) { - m_inputChain = new TChain(m_cfg.treeName.c_str()); + m_inputChain = std::make_unique(m_cfg.treeName.c_str()); if (m_cfg.filePath.empty()) { throw std::invalid_argument("Missing input filename"); @@ -103,6 +103,8 @@ RootSimHitReader::RootSimHitReader(const RootSimHitReader::Config& config, << availableEvents().second); } +RootSimHitReader::~RootSimHitReader() = default; + std::pair RootSimHitReader::availableEvents() const { return {std::get<0>(m_eventMap.front()), std::get<0>(m_eventMap.back()) + 1}; } diff --git a/Examples/Io/Root/src/RootTrackSummaryReader.cpp b/Examples/Io/Root/src/RootTrackSummaryReader.cpp index 9ff3590271a..33da0026ef7 100644 --- a/Examples/Io/Root/src/RootTrackSummaryReader.cpp +++ b/Examples/Io/Root/src/RootTrackSummaryReader.cpp @@ -31,7 +31,7 @@ RootTrackSummaryReader::RootTrackSummaryReader( : IReader(), m_logger{Acts::getDefaultLogger(name(), level)}, m_cfg(config) { - m_inputChain = new TChain(m_cfg.treeName.c_str()); + m_inputChain = std::make_unique(m_cfg.treeName.c_str()); if (m_cfg.filePath.empty()) { throw std::invalid_argument("Missing input filename"); diff --git a/Examples/Io/Root/src/RootVertexReader.cpp b/Examples/Io/Root/src/RootVertexReader.cpp index 31a8e6d5bec..d2d2848749d 100644 --- a/Examples/Io/Root/src/RootVertexReader.cpp +++ b/Examples/Io/Root/src/RootVertexReader.cpp @@ -28,7 +28,7 @@ RootVertexReader::RootVertexReader(const RootVertexReader::Config& config, : IReader(), m_cfg(config), m_logger(Acts::getDefaultLogger(name(), level)) { - m_inputChain = new TChain(m_cfg.treeName.c_str()); + m_inputChain = std::make_unique(m_cfg.treeName.c_str()); if (m_cfg.filePath.empty()) { throw std::invalid_argument("Missing input filename"); diff --git a/Examples/Io/Root/src/detail/NuclearInteractionParametrisation.cpp b/Examples/Io/Root/src/detail/NuclearInteractionParametrisation.cpp index 174f3648ce1..eb1bbe838e8 100644 --- a/Examples/Io/Root/src/detail/NuclearInteractionParametrisation.cpp +++ b/Examples/Io/Root/src/detail/NuclearInteractionParametrisation.cpp @@ -32,15 +32,20 @@ namespace { /// @return The location in a standard normal distribution float gaussianValue(TH1F const* histo, const float mom) { // Get the cumulative probability distribution - TH1F* normalised = static_cast(histo->DrawNormalized()); - TH1F* cumulative = static_cast(normalised->GetCumulative()); + + // DrawNormalized and GetCumulative return pointers to histograms where ROOT + // transfers ownership to the caller. + std::unique_ptr normalised( + dynamic_cast(histo->DrawNormalized())); + std::unique_ptr cumulative( + dynamic_cast(normalised->GetCumulative())); + assert(cumulative); + assert(normalised); // Find the cumulative probability const float binContent = cumulative->GetBinContent(cumulative->FindBin(mom)); // Transform the probability to an entry in a standard normal distribution const float value = TMath::ErfInverse(2. * binContent - 1.); - delete (normalised); - delete (cumulative); return value; } From edad4c94bd83e100419d4a6d2f40f603fceaab7e Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Fri, 22 Nov 2024 13:50:15 +0100 Subject: [PATCH 2/2] feat(util): Add `overloaded` helper (#3816) This is useful in combination with `std::visit`. With this, you can do something like ```cpp struct A {}; std::variant var; var = 42; std::visit(overloaded{ [](int) { BOOST_CHECK(true); }, [](double) { BOOST_CHECK(false); }, [](A) { BOOST_CHECK(false); }, }, var); ``` which can be convenient. I'm using this in https://github.com/acts-project/acts/pull/3869 --- Core/include/Acts/Utilities/Helpers.hpp | 17 +++++++++++++++++ Tests/UnitTests/Core/Utilities/HelpersTests.cpp | 15 +++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/Core/include/Acts/Utilities/Helpers.hpp b/Core/include/Acts/Utilities/Helpers.hpp index 4bf9d1c89c3..c59555fd848 100644 --- a/Core/include/Acts/Utilities/Helpers.hpp +++ b/Core/include/Acts/Utilities/Helpers.hpp @@ -204,4 +204,21 @@ bool rangeContainsValue(const R& range, const T& value) { return std::ranges::find(range, value) != std::ranges::end(range); } +/// Helper struct that can turn a set of lambdas into a single entity with +/// overloaded call operator. This can be useful for example in a std::visit +/// call. +/// ```cpp +/// std::visit(overloaded{ +/// [](const int& i) { std::cout << "int: " << i << std::endl; }, +/// [](const std::string& s) { std::cout << "string: " << s << std::endl; }, +/// }, variant); +/// ``` +template +struct overloaded : Ts... { + using Ts::operator()...; +}; + +template +overloaded(Ts...) -> overloaded; + } // namespace Acts diff --git a/Tests/UnitTests/Core/Utilities/HelpersTests.cpp b/Tests/UnitTests/Core/Utilities/HelpersTests.cpp index 85e8454d052..67064306967 100644 --- a/Tests/UnitTests/Core/Utilities/HelpersTests.cpp +++ b/Tests/UnitTests/Core/Utilities/HelpersTests.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include using namespace Acts::VectorHelpers; @@ -310,6 +311,20 @@ BOOST_AUTO_TEST_CASE(incidentAnglesTest) { } } +BOOST_AUTO_TEST_CASE(Overloaded) { + struct A {}; + std::variant var; + + var = 42; + + std::visit(overloaded{ + [](int) { BOOST_CHECK(true); }, + [](double) { BOOST_CHECK(false); }, + [](A) { BOOST_CHECK(false); }, + }, + var); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace Acts::Test