Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Avoid new/delete in a number of places. #3828

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

refactor: Avoid new/delete in a number of places. #3828

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bfaa94a
SurfaceArrayCreator
paulgessinger Nov 7, 2024
61cb843
GBTS
paulgessinger Nov 7, 2024
0852e57
ProtoLayerCreatorT
paulgessinger Nov 7, 2024
9501894
ROOT IO
paulgessinger Nov 7, 2024
8f7b4c5
LayerCreator
paulgessinger Nov 7, 2024
337979f
fix compilation
paulgessinger Nov 11, 2024
d1f3a22
revert doc comment change
paulgessinger Nov 12, 2024
ac272ee
Merge remote-tracking branch 'origin/main' into refactor/avoid-new-de…
paulgessinger Nov 12, 2024
b48d00f
fix another compile issue
paulgessinger Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 9 additions & 25 deletions Core/include/Acts/Geometry/SurfaceArrayCreator.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,23 +130,7 @@ class SurfaceArrayCreator {
std::size_t binsZ, std::optional<ProtoLayer> protoLayerOpt = std::nullopt,
const Transform3& transform = Transform3::Identity()) const;

/// SurfaceArrayCreator interface method
///
/// - create an array in a cylinder, binned in phi, z when extremas and bin
/// numbers are unknown - this method goes through the surfaces and finds out
/// the needed information
/// @warning This function requires the cylinder aligned with the z-axis
/// @param surfaces is the vector of pointers to sensitive surfaces
/// to be ordered on the cylinder
/// @pre the pointers to the sensitive surfaces in the surfaces vectors all
/// need to be valid, since no check is performed
/// @param [in] gctx The gometry context for this building call
/// @param protoLayerOpt The proto layer containing the layer size
/// @param bTypePhi the binning type in phi direction (equidistant/arbitrary)
/// @param bTypeZ the binning type in z direction (equidistant/arbitrary)
/// @param transform is the (optional) additional transform applied
///
/// @return a unique pointer a new SurfaceArray
/// SurfaceArrayCreator interface methonew SurfaceArray
andiwand marked this conversation as resolved.
Show resolved Hide resolved
std::unique_ptr<Acts::SurfaceArray> surfaceArrayOnCylinder(
const GeometryContext& gctx,
std::vector<std::shared_ptr<const Surface>> surfaces,
Expand Down Expand Up @@ -380,35 +364,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
54 changes: 19 additions & 35 deletions Core/include/Acts/Seeding/GbtsDataStorage.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,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 @@ -96,27 +89,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 <= M_PI - dphi) {
continue;
}
Expand All @@ -126,14 +112,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 >= -M_PI + dphi) {
break;
}
Expand All @@ -142,9 +128,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 @@ -185,16 +169,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 @@ -217,16 +203,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
37 changes: 11 additions & 26 deletions Core/include/Acts/Seeding/GbtsGeometry.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,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 +296,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 +317,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 +328,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 +341,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
Open in desktop
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
Open in desktop
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
Open in desktop
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