Skip to content

Commit

Permalink
refactor: Avoid new/delete in a number of places. (#3828)
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulgessinger
paulgessinger authored Nov 22, 2024
1 parent c3c4bf5 commit 6e3a9a6
Show file tree
Hide file tree
Showing 23 changed files with 100 additions and 143 deletions.
16 changes: 8 additions & 8 deletions Core/include/Acts/Geometry/SurfaceArrayCreator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,35 +381,35 @@ class SurfaceArrayCreator {
Axis<AxisType::Equidistant, bdtB> axisB(pAxisB.min, pAxisB.max, pAxisB.nBins);

using SGL = SurfaceArray::SurfaceGridLookup<decltype(axisA), decltype(axisB)>;
ptr = std::unique_ptr<ISGL>(static_cast<ISGL*>(
new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue})));
ptr = std::make_unique<SGL>(
globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue});

} else if (pAxisA.bType == equidistant && pAxisB.bType == arbitrary) {

Axis<AxisType::Equidistant, bdtA> axisA(pAxisA.min, pAxisA.max, pAxisA.nBins);
Axis<AxisType::Variable, bdtB> axisB(pAxisB.binEdges);

using SGL = SurfaceArray::SurfaceGridLookup<decltype(axisA), decltype(axisB)>;
ptr = std::unique_ptr<ISGL>(static_cast<ISGL*>(
new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue})));
ptr = std::make_unique<SGL>(
globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue});

} else if (pAxisA.bType == arbitrary && pAxisB.bType == equidistant) {

Axis<AxisType::Variable, bdtA> axisA(pAxisA.binEdges);
Axis<AxisType::Equidistant, bdtB> axisB(pAxisB.min, pAxisB.max, pAxisB.nBins);

using SGL = SurfaceArray::SurfaceGridLookup<decltype(axisA), decltype(axisB)>;
ptr = std::unique_ptr<ISGL>(static_cast<ISGL*>(
new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue})));
ptr = std::make_unique<SGL>(
globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue});

} else /*if (pAxisA.bType == arbitrary && pAxisB.bType == arbitrary)*/ {

Axis<AxisType::Variable, bdtA> axisA(pAxisA.binEdges);
Axis<AxisType::Variable, bdtB> axisB(pAxisB.binEdges);

using SGL = SurfaceArray::SurfaceGridLookup<decltype(axisA), decltype(axisB)>;
ptr = std::unique_ptr<ISGL>(static_cast<ISGL*>(
new SGL(globalToLocal, localToGlobal, std::make_tuple(axisA, axisB), {pAxisA.bValue, pAxisB.bValue})));
ptr = std::make_unique<SGL>(
globalToLocal, localToGlobal, std::pair{axisA, axisB}, std::vector{pAxisA.bValue, pAxisB.bValue});
}
// clang-format on

Expand Down
55 changes: 20 additions & 35 deletions Core/include/Acts/Seeding/GbtsDataStorage.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "Acts/Seeding/GbtsGeometry.hpp"

#include <algorithm>
#include <iostream>
#include <map>
#include <numbers>
#include <vector>
Expand Down Expand Up @@ -47,13 +48,6 @@ struct GbtsSP {
template <typename space_point_t>
class GbtsNode {
public:
struct CompareByPhi {
bool operator()(const GbtsNode<space_point_t> *n1,
const GbtsNode<space_point_t> *n2) {
return (n1->m_spGbts.phi() < n2->m_spGbts.phi());
}
};

GbtsNode(const GbtsSP<space_point_t> &spGbts, float minT = -100.0,
float maxT = 100.0)
: m_spGbts(spGbts), m_minCutOnTau(minT), m_maxCutOnTau(maxT) {}
Expand Down Expand Up @@ -97,27 +91,20 @@ class GbtsEtaBin {
public:
GbtsEtaBin() { m_vn.clear(); }

~GbtsEtaBin() {
for (typename std::vector<GbtsNode<space_point_t> *>::iterator it =
m_vn.begin();
it != m_vn.end(); ++it) {
delete (*it);
}
}

void sortByPhi() {
std::ranges::sort(m_vn,
typename Acts::GbtsNode<space_point_t>::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<space_point_t> *pN = m_vn.at(nIdx);
GbtsNode<space_point_t> &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<float> - dphi) {
continue;
}
Expand All @@ -127,14 +114,14 @@ class GbtsEtaBin {
}

for (unsigned int nIdx = 0; nIdx < m_vn.size(); nIdx++) {
GbtsNode<space_point_t> *pN = m_vn.at(nIdx);
float phi = pN->m_spGbts.phi();
GbtsNode<space_point_t> &pN = *m_vn.at(nIdx);
float phi = pN.m_spGbts.phi();
m_vPhiNodes.push_back(std::pair<float, unsigned int>(phi, nIdx));
}

for (unsigned int nIdx = 0; nIdx < m_vn.size(); nIdx++) {
GbtsNode<space_point_t> *pN = m_vn.at(nIdx);
float phi = pN->m_spGbts.phi();
GbtsNode<space_point_t> &pN = *m_vn.at(nIdx);
float phi = pN.m_spGbts.phi();
if (phi >= -std::numbers::pi_v<float> + dphi) {
break;
}
Expand All @@ -143,9 +130,7 @@ class GbtsEtaBin {
}
}

std::vector<GbtsNode<space_point_t> *> m_vn;
// TODO change to
// std::vector<std::unique_ptr<GbtsNode<space_point_t>>> m_vn;
std::vector<std::unique_ptr<GbtsNode<space_point_t>>> m_vn;
std::vector<std::pair<float, unsigned int>> m_vPhiNodes;
};

Expand Down Expand Up @@ -186,16 +171,18 @@ 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<space_point_t>(
sp, min_tau, max_tau)); // adding ftf member to nodes
m_etaBins.at(binIndex).m_vn.push_back(
std::make_unique<GbtsNode<space_point_t>>(
sp, min_tau, max_tau)); // adding ftf member to nodes
} else {
if (useClusterWidth) {
float cluster_width = 1; // temporary while cluster width not available
if (cluster_width > 0.2) {
return -3;
}
}
m_etaBins.at(binIndex).m_vn.push_back(new GbtsNode<space_point_t>(sp));
m_etaBins.at(binIndex).m_vn.push_back(
std::make_unique<GbtsNode<space_point_t>>(sp));
}

return 0;
Expand All @@ -218,16 +205,14 @@ class GbtsDataStorage {
vn.clear();
vn.reserve(numberOfNodes());
for (const auto &b : m_etaBins) {
for (typename std::vector<GbtsNode<space_point_t> *>::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());
}
}
}
Expand Down
38 changes: 12 additions & 26 deletions Core/include/Acts/Seeding/GbtsGeometry.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <algorithm>
#include <cmath>
#include <iostream>
#include <map>
#include <memory>
#include <vector>
Expand Down Expand Up @@ -277,15 +278,10 @@ class GbtsGeometry {

// calculating bin tables in the connector...

for (std::map<int, std::vector<GbtsConnection *>>::const_iterator it =
m_connector->m_connMap.begin();
it != m_connector->m_connMap.end(); ++it) {
const std::vector<GbtsConnection *> &vConn = (*it).second;

for (std::vector<GbtsConnection *>::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<space_point_t> *pL1 = getGbtsLayerByKey(dst);
const GbtsLayer<space_point_t> *pL2 = getGbtsLayerByKey(src);
Expand All @@ -301,15 +297,15 @@ 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
if (!pL1->verifyBin(pL2, b1, b2, min_z0, max_z0)) {
continue;
}
int address = b1 + b2 * nDstBins;
(*cIt)->m_binTable.at(address) = 1;
c->m_binTable.at(address) = 1;
}
}
}
Expand All @@ -322,17 +318,6 @@ class GbtsGeometry {
GbtsGeometry(const GbtsGeometry &) = delete;
GbtsGeometry &operator=(const GbtsGeometry &) = delete;

~GbtsGeometry() {
for (typename std::vector<GbtsLayer<space_point_t> *>::iterator it =
m_layArray.begin();
it != m_layArray.end(); ++it) {
delete (*it);
}

m_layMap.clear();
m_layArray.clear();
}

const GbtsLayer<space_point_t> *getGbtsLayerByKey(unsigned int key) const {
typename std::map<unsigned int, GbtsLayer<space_point_t> *>::const_iterator
it = m_layMap.find(key);
Expand All @@ -344,7 +329,7 @@ class GbtsGeometry {
}

const GbtsLayer<space_point_t> *getGbtsLayerByIndex(int idx) const {
return m_layArray.at(idx);
return m_layArray.at(idx).get();
}

int num_bins() const { return m_nEtaBins; }
Expand All @@ -357,18 +342,19 @@ class GbtsGeometry {
unsigned int layerKey = l.m_subdet; // this should be combined ID
float ew = m_etaBinWidth;

GbtsLayer<space_point_t> *pHL = new GbtsLayer<space_point_t>(l, ew, bin0);
auto upHL = std::make_unique<GbtsLayer<space_point_t>>(l, ew, bin0);
auto *pHL = upHL.get();

m_layMap.insert(
std::pair<unsigned int, GbtsLayer<space_point_t> *>(layerKey, pHL));
m_layArray.push_back(pHL);
m_layArray.push_back(std::move(upHL));
return pHL;
}

float m_etaBinWidth{};

std::map<unsigned int, GbtsLayer<space_point_t> *> m_layMap;
std::vector<GbtsLayer<space_point_t> *> m_layArray;
std::vector<std::unique_ptr<GbtsLayer<space_point_t>>> m_layArray;

int m_nEtaBins{0};

Expand Down
28 changes: 11 additions & 17 deletions Core/include/Acts/Seeding/SeedFinderGbts.ipp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,7 @@ void SeedFinderGbts<external_spacepoint_t>::runGbts_TrackFinder(
}

unsigned int first_it = 0;
for (typename std::vector<
GbtsNode<external_spacepoint_t>*>::const_iterator n1It =
B1.m_vn.begin();
n1It != B1.m_vn.end(); ++n1It) { // loop over nodes in Layer 1

GbtsNode<external_spacepoint_t>* 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;
Expand Down Expand Up @@ -195,7 +190,7 @@ void SeedFinderGbts<external_spacepoint_t>::runGbts_TrackFinder(
}

GbtsNode<external_spacepoint_t>* 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;
Expand Down Expand Up @@ -304,8 +299,8 @@ void SeedFinderGbts<external_spacepoint_t>::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);
Expand All @@ -332,21 +327,20 @@ void SeedFinderGbts<external_spacepoint_t>::runGbts_TrackFinder(
int nNodes = vNodes.size();

for (int nodeIdx = 0; nodeIdx < nNodes; nodeIdx++) {
const GbtsNode<external_spacepoint_t>* pN = vNodes.at(nodeIdx);
const GbtsNode<external_spacepoint_t>& pN = *vNodes.at(nodeIdx);

std::vector<std::pair<float, int>> 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<int>(pN->m_in.size()); inIdx++) {
int inEdgeIdx = pN->m_in.at(inIdx);
for (int inIdx = 0; inIdx < static_cast<int>(pN.m_in.size()); inIdx++) {
int inEdgeIdx = pN.m_in.at(inIdx);
Acts::GbtsEdge<external_spacepoint_t>* pS = &(edgeStorage.at(inEdgeIdx));
in_sort[inIdx].second = inEdgeIdx;
in_sort[inIdx].first = pS->m_p[0];
}
for (int outIdx = 0; outIdx < static_cast<int>(pN->m_out.size());
outIdx++) {
int outEdgeIdx = pN->m_out.at(outIdx);
for (int outIdx = 0; outIdx < static_cast<int>(pN.m_out.size()); outIdx++) {
int outEdgeIdx = pN.m_out.at(outIdx);
Acts::GbtsEdge<external_spacepoint_t>* pS = &(edgeStorage.at(outEdgeIdx));
out_sort[outIdx].second = outEdgeIdx;
out_sort[outIdx].first = pS->m_p[0];
Expand Down
9 changes: 2 additions & 7 deletions Core/include/Acts/TrackFinding/GbtsConnector.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
// TODO: update to C++17 style
// Consider to moving to detail subdirectory
#include <fstream>
#include <iostream>
#include <map>
#include <memory>
#include <vector>

namespace Acts {
Expand All @@ -39,15 +39,10 @@ class GbtsConnector {

GbtsConnector(std::ifstream &inFile);

~GbtsConnector();

float m_etaBin{};

std::map<int, std::vector<struct LayerGroup>> m_layerGroups;
std::map<int, std::vector<Acts::GbtsConnection *>> m_connMap;
// TODO: change to std::map<int, std::vector<Acts::GbtsConnection> >
// m_connMap; or std::map<int,
// std::vector<std::unique_ptr<Acts::GbtsConnection>> > m_connMap;
std::map<int, std::vector<std::unique_ptr<Acts::GbtsConnection>>> m_connMap;
};

} // namespace Acts
3 changes: 1 addition & 2 deletions Core/src/Geometry/LayerCreator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,8 +407,7 @@ Acts::MutableLayerPtr Acts::LayerCreator::planeLayer(
}

// create the layer and push it back
std::shared_ptr<const PlanarBounds> pBounds(
new RectangleBounds(layerHalf1, layerHalf2));
auto pBounds = std::make_shared<RectangleBounds>(layerHalf1, layerHalf2);

// create the layer
MutableLayerPtr pLayer =
Expand Down
Loading

0 comments on commit 6e3a9a6

Please sign in to comment.