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.

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

refactor!: Using non const InternalSpacePoint objects #1196

Merged
merged 8 commits into from
Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
4 changes: 2 additions & 2 deletions Core/include/Acts/Seeding/BinnedSPGroup.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ template <typename external_spacepoint_t>
class NeighborhoodIterator {
public:
using sp_it_t = typename std::vector<std::unique_ptr<
const InternalSpacePoint<external_spacepoint_t>>>::const_iterator;
InternalSpacePoint<external_spacepoint_t>>>::const_iterator;

NeighborhoodIterator() = delete;

Expand Down Expand Up @@ -94,7 +94,7 @@ class NeighborhoodIterator {
}
}

const InternalSpacePoint<external_spacepoint_t>* operator*() {
InternalSpacePoint<external_spacepoint_t>* operator*() {
return (*m_curIt).get();
}

Expand Down
12 changes: 5 additions & 7 deletions Core/include/Acts/Seeding/BinnedSPGroup.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ Acts::BinnedSPGroup<external_spacepoint_t>::BinnedSPGroup(
// create number of bins equal to number of millimeters rMax
// (worst case minR: configured minR + 1mm)
size_t numRBins = (config.rMax + config.beamPos.norm());
std::vector<std::vector<
std::unique_ptr<const InternalSpacePoint<external_spacepoint_t>>>>
std::vector<
std::vector<std::unique_ptr<InternalSpacePoint<external_spacepoint_t>>>>
rBins(numRBins);
for (spacepoint_iterator_t it = spBegin; it != spEnd; it++) {
if (*it == nullptr) {
Expand All @@ -58,9 +58,8 @@ Acts::BinnedSPGroup<external_spacepoint_t>::BinnedSPGroup(
continue;
}

auto isp =
std::make_unique<const InternalSpacePoint<external_spacepoint_t>>(
sp, spPosition, config.beamPos, variance);
auto isp = std::make_unique<InternalSpacePoint<external_spacepoint_t>>(
sp, spPosition, config.beamPos, variance);
// calculate r-Bin index and protect against overflow (underflow not
// possible)
size_t rIndex = isp->radius();
Expand All @@ -75,8 +74,7 @@ Acts::BinnedSPGroup<external_spacepoint_t>::BinnedSPGroup(
for (auto& rbin : rBins) {
for (auto& isp : rbin) {
Acts::Vector2 spLocation(isp->phi(), isp->z());
std::vector<
std::unique_ptr<const InternalSpacePoint<external_spacepoint_t>>>&
std::vector<std::unique_ptr<InternalSpacePoint<external_spacepoint_t>>>&
bin = grid->atPosition(spLocation);
bin.push_back(std::move(isp));
}
Expand Down
13 changes: 6 additions & 7 deletions Core/include/Acts/Seeding/InternalSeed.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ class InternalSeed {
/////////////////////////////////////////////////////////////////////////////////

public:
InternalSeed(const InternalSpacePoint<SpacePoint>& s0,
const InternalSpacePoint<SpacePoint>& s1,
const InternalSpacePoint<SpacePoint>& s2, float z);
InternalSeed(InternalSpacePoint<SpacePoint>& s0,
InternalSpacePoint<SpacePoint>& s1,
InternalSpacePoint<SpacePoint>& s2, float z);
InternalSeed& operator=(const InternalSeed& seed);

const std::array<const InternalSpacePoint<SpacePoint>*, 3> sp;
const std::array<InternalSpacePoint<SpacePoint>*, 3> sp;
float z() const { return m_z; }

protected:
Expand All @@ -49,9 +49,8 @@ inline InternalSeed<SpacePoint>& InternalSeed<SpacePoint>::operator=(

template <typename SpacePoint>
inline InternalSeed<SpacePoint>::InternalSeed(
const InternalSpacePoint<SpacePoint>& s0,
const InternalSpacePoint<SpacePoint>& s1,
const InternalSpacePoint<SpacePoint>& s2, float z)
InternalSpacePoint<SpacePoint>& s0, InternalSpacePoint<SpacePoint>& s1,
InternalSpacePoint<SpacePoint>& s2, float z)
: sp({&s0, &s1, &s2}) {
m_z = z;
}
Expand Down
40 changes: 24 additions & 16 deletions Core/include/Acts/Seeding/InternalSpacePoint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,32 @@ class InternalSpacePoint {
float phi() const { return atan2f(m_y, m_x); }
const float& varianceR() const { return m_varianceR; }
const float& varianceZ() const { return m_varianceZ; }
const float& quality() const { return m_quality; }
const float& cotTheta() const { return m_cotTheta; }
void setCotTheta(float cotTheta) { m_cotTheta = cotTheta; }
void setQuality(float quality) {
if (quality >= m_quality) {
m_quality = quality;
}
}
const SpacePoint& sp() const { return m_sp; }

protected:
float m_x; // x-coordinate in beam system coordinates
float m_y; // y-coordinate in beam system coordinates
float m_z; // z-coordinate in beam system coordinetes
float m_r; // radius in beam system coordinates
float m_varianceR; //
float m_varianceZ; //
const SpacePoint& m_sp; // external space point
float m_x; // x-coordinate in beam system coordinates
float m_y; // y-coordinate in beam system coordinates
float m_z; // z-coordinate in beam system coordinetes
float m_r; // radius in beam system coordinates
float m_varianceR; //
float m_varianceZ; //
float m_cotTheta = std::numeric_limits<
double>::quiet_NaN(); // 1/tanTheta estimated from central+this space
// point. Its evaluation requires that the space
// point is a candidate for triplet search.
float m_quality = -std::numeric_limits<
double>::infinity(); // Quality score of the seed the space point is used
// for. Quality can be changed if the space point is
// used for a better quality seed.
const SpacePoint& m_sp; // external space point
};

/////////////////////////////////////////////////////////////////////////////////
Expand All @@ -75,14 +91,6 @@ inline InternalSpacePoint<SpacePoint>::InternalSpacePoint(

template <typename SpacePoint>
inline InternalSpacePoint<SpacePoint>::InternalSpacePoint(
const InternalSpacePoint<SpacePoint>& sp)
: m_sp(sp.sp()) {
m_x = sp.m_x;
m_y = sp.m_y;
m_z = sp.m_z;
m_r = sp.m_r;
m_varianceR = sp.m_varianceR;
m_varianceZ = sp.m_varianceZ;
}
const InternalSpacePoint<SpacePoint>& sp) = default;

} // end of namespace Acts
6 changes: 3 additions & 3 deletions Core/include/Acts/Seeding/SeedFilter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ class SeedFilter {
/// @param zOrigin on the z axis as defined by bottom and middle space point
/// @param outIt Output iterator for the seeds
virtual void filterSeeds_2SpFixed(
const InternalSpacePoint<external_spacepoint_t>& bottomSP,
const InternalSpacePoint<external_spacepoint_t>& middleSP,
std::vector<const InternalSpacePoint<external_spacepoint_t>*>& topSpVec,
InternalSpacePoint<external_spacepoint_t>& bottomSP,
InternalSpacePoint<external_spacepoint_t>& middleSP,
std::vector<InternalSpacePoint<external_spacepoint_t>*>& topSpVec,
std::vector<float>& invHelixDiameterVec,
std::vector<float>& impactParametersVec, float zOrigin,
std::back_insert_iterator<std::vector<std::pair<
Expand Down
12 changes: 9 additions & 3 deletions Core/include/Acts/Seeding/SeedFilter.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ SeedFilter<external_spacepoint_t>::SeedFilter(
// return vector must contain weight of each seed
template <typename external_spacepoint_t>
void SeedFilter<external_spacepoint_t>::filterSeeds_2SpFixed(
const InternalSpacePoint<external_spacepoint_t>& bottomSP,
const InternalSpacePoint<external_spacepoint_t>& middleSP,
std::vector<const InternalSpacePoint<external_spacepoint_t>*>& topSpVec,
InternalSpacePoint<external_spacepoint_t>& bottomSP,
InternalSpacePoint<external_spacepoint_t>& middleSP,
std::vector<InternalSpacePoint<external_spacepoint_t>*>& topSpVec,
std::vector<float>& invHelixDiameterVec,
std::vector<float>& impactParametersVec, float zOrigin,
std::back_insert_iterator<std::vector<std::pair<
Expand Down Expand Up @@ -141,6 +141,12 @@ void SeedFilter<external_spacepoint_t>::filterSeeds_1SpFixed(
// ordering by weight by filterSeeds_2SpFixed means these are the lowest
// weight seeds
for (; it < itBegin + maxSeeds; ++it) {
float bestSeedQuality = (*it).first;

(*it).second->sp[0]->setQuality(bestSeedQuality);
(*it).second->sp[1]->setQuality(bestSeedQuality);
(*it).second->sp[2]->setQuality(bestSeedQuality);

outIt = Seed<external_spacepoint_t>{
(*it).second->sp[0]->sp(), (*it).second->sp[1]->sp(),
(*it).second->sp[2]->sp(), (*it).second->z()};
Expand Down
10 changes: 5 additions & 5 deletions Core/include/Acts/Seeding/SeedFinderUtils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ struct LinCircle {
/// @param[in] spM The middle spacepoint to use.
/// @param[in] bottom Should be true if sp is a bottom SP.
template <typename external_spacepoint_t>
LinCircle transformCoordinates(
const InternalSpacePoint<external_spacepoint_t>& sp,
const InternalSpacePoint<external_spacepoint_t>& spM, bool bottom);
LinCircle transformCoordinates(InternalSpacePoint<external_spacepoint_t>& sp,
InternalSpacePoint<external_spacepoint_t>& spM,
bool bottom);

/// @brief Transform a vector of spacepoints to u-v space circles with respect
/// to a given middle spacepoint.
Expand All @@ -48,8 +48,8 @@ LinCircle transformCoordinates(
/// @param[out] linCircleVec The output vector to write to.
template <typename external_spacepoint_t>
void transformCoordinates(
const std::vector<const InternalSpacePoint<external_spacepoint_t>*>& vec,
const InternalSpacePoint<external_spacepoint_t>& spM, bool bottom,
const std::vector<InternalSpacePoint<external_spacepoint_t>*>& vec,
InternalSpacePoint<external_spacepoint_t>& spM, bool bottom,
bool enableCutsForSortedSP, std::vector<LinCircle>& linCircleVec);
} // namespace Acts

Expand Down
11 changes: 6 additions & 5 deletions Core/include/Acts/Seeding/SeedFinderUtils.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

namespace Acts {
template <typename external_spacepoint_t>
LinCircle transformCoordinates(
const InternalSpacePoint<external_spacepoint_t>& sp,
const InternalSpacePoint<external_spacepoint_t>& spM, bool bottom) {
LinCircle transformCoordinates(InternalSpacePoint<external_spacepoint_t>& sp,
InternalSpacePoint<external_spacepoint_t>& spM,
bool bottom) {
// The computation inside this function is exactly identical to that in the
// vectorized version of this function, except that it operates on a single
// spacepoint. Please see the other version of this function for more
Expand Down Expand Up @@ -46,8 +46,8 @@ LinCircle transformCoordinates(

template <typename external_spacepoint_t>
void transformCoordinates(
const std::vector<const InternalSpacePoint<external_spacepoint_t>*>& vec,
const InternalSpacePoint<external_spacepoint_t>& spM, bool bottom,
const std::vector<InternalSpacePoint<external_spacepoint_t>*>& vec,
InternalSpacePoint<external_spacepoint_t>& spM, bool bottom,
bool enableCutsForSortedSP, std::vector<LinCircle>& linCircleVec) {
float xM = spM.x();
float yM = spM.y();
Expand Down Expand Up @@ -93,6 +93,7 @@ void transformCoordinates(
(cot_theta * cot_theta) * (varianceRM + sp->varianceR())) *
iDeltaR2;
linCircleVec.push_back(l);
sp->setCotTheta(cot_theta);
}
// sort the SP in order of cotTheta
if (enableCutsForSortedSP) {
Expand Down
7 changes: 3 additions & 4 deletions Core/include/Acts/Seeding/Seedfinder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,16 @@ class Seedfinder {
public:
struct State {
// bottom space point
std::vector<const InternalSpacePoint<external_spacepoint_t>*>
compatBottomSP;
std::vector<const InternalSpacePoint<external_spacepoint_t>*> compatTopSP;
std::vector<InternalSpacePoint<external_spacepoint_t>*> compatBottomSP;
std::vector<InternalSpacePoint<external_spacepoint_t>*> compatTopSP;
// contains parameters required to calculate circle with linear equation
// ...for bottom-middle
std::vector<LinCircle> linCircleBottom;
// ...for middle-top
std::vector<LinCircle> linCircleTop;

// create vectors here to avoid reallocation in each loop
std::vector<const InternalSpacePoint<external_spacepoint_t>*> topSpVec;
std::vector<InternalSpacePoint<external_spacepoint_t>*> topSpVec;
std::vector<float> curvatures;
std::vector<float> impactParameters;
std::vector<float> etaVec;
Expand Down
3 changes: 1 addition & 2 deletions Core/include/Acts/Seeding/SpacePointGrid.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ struct SpacePointGridConfig {

template <typename external_spacepoint_t>
using SpacePointGrid = detail::Grid<
std::vector<
std::unique_ptr<const InternalSpacePoint<external_spacepoint_t>>>,
std::vector<std::unique_ptr<InternalSpacePoint<external_spacepoint_t>>>,
detail::Axis<detail::AxisType::Equidistant,
detail::AxisBoundaryType::Closed>,
detail::Axis<detail::AxisType::Variable, detail::AxisBoundaryType::Bound>>;
Expand Down
10 changes: 4 additions & 6 deletions Plugins/Cuda/include/Acts/Plugins/Cuda/Seeding/Seedfinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,9 @@ Seedfinder<external_spacepoint_t, Acts::Cuda>::createSeedsForGroup(
// Algorithm 0. Matrix Flattening
//---------------------------------

std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*>
middleSPvec;
std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*>
bottomSPvec;
std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*> topSPvec;
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> middleSPvec;
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> bottomSPvec;
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> topSPvec;

// Get the size of spacepoints
int nSpM(0);
Expand Down Expand Up @@ -296,4 +294,4 @@ Seedfinder<external_spacepoint_t, Acts::Cuda>::createSeedsForGroup(
ACTS_CUDA_ERROR_CHECK(cudaStreamDestroy(cuStream));
return outputVec;
}
} // namespace Acts
} // namespace Acts
20 changes: 10 additions & 10 deletions Plugins/Cuda/include/Acts/Plugins/Cuda/Seeding2/SeedFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,18 @@ SeedFinder<external_spacepoint_t>::createSeedsForGroup(

// Create more convenient vectors out of the space point containers.
auto spVecMaker = [](sp_range_t spRange) {
std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*> result;
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> result;
for (auto* sp : spRange) {
result.push_back(sp);
}
return result;
};

std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*>
bottomSPVec(spVecMaker(bottomSPs));
std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*>
middleSPVec(spVecMaker(middleSPs));
std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*> topSPVec(
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> bottomSPVec(
spVecMaker(bottomSPs));
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> middleSPVec(
spVecMaker(middleSPs));
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> topSPVec(
spVecMaker(topSPs));

// If either one of them is empty, we have nothing to find.
Expand Down Expand Up @@ -193,12 +193,12 @@ SeedFinder<external_spacepoint_t>::createSeedsForGroup(
std::vector<std::pair<
float, std::unique_ptr<const InternalSeed<external_spacepoint_t>>>>
seedsPerSPM;
const auto& middleSP = *(middleSPVec[middleIndex]);
auto& middleSP = *(middleSPVec[middleIndex]);
for (const Details::Triplet& triplet : *triplet_itr) {
assert(triplet.bottomIndex < bottomSPVec.size());
const auto& bottomSP = *(bottomSPVec[triplet.bottomIndex]);
auto& bottomSP = *(bottomSPVec[triplet.bottomIndex]);
assert(triplet.topIndex < topSPVec.size());
const auto& topSP = *(topSPVec[triplet.topIndex]);
auto& topSP = *(topSPVec[triplet.topIndex]);
seedsPerSPM.emplace_back(std::make_pair(
triplet.weight,
std::make_unique<const InternalSeed<external_spacepoint_t>>(
Expand All @@ -222,4 +222,4 @@ void SeedFinder<external_spacepoint_t>::setLogger(
}

} // namespace Cuda
} // namespace Acts
} // namespace Acts
24 changes: 10 additions & 14 deletions Plugins/Sycl/include/Acts/Plugins/Sycl/Seeding/Seedfinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -84,37 +84,33 @@ Seedfinder<external_spacepoint_t>::createSeedsForGroup(
vecmem::vector<detail::DeviceSpacePoint> deviceMiddleSPs(m_resource);
vecmem::vector<detail::DeviceSpacePoint> deviceTopSPs(m_resource);

std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*>
bottomSPvec;
std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*>
middleSPvec;
std::vector<const Acts::InternalSpacePoint<external_spacepoint_t>*> topSPvec;
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> bottomSPvec;
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> middleSPvec;
std::vector<Acts::InternalSpacePoint<external_spacepoint_t>*> topSPvec;

for (const Acts::InternalSpacePoint<external_spacepoint_t>* SP : bottomSPs) {
for (auto SP : bottomSPs) {
bottomSPvec.push_back(SP);
}
deviceBottomSPs.reserve(bottomSPvec.size());
for (const Acts::InternalSpacePoint<external_spacepoint_t>* SP :
bottomSPvec) {
for (auto SP : bottomSPvec) {
deviceBottomSPs.push_back({SP->x(), SP->y(), SP->z(), SP->radius(),
SP->varianceR(), SP->varianceZ()});
}

for (const Acts::InternalSpacePoint<external_spacepoint_t>* SP : middleSPs) {
for (auto SP : middleSPs) {
middleSPvec.push_back(SP);
}
deviceMiddleSPs.reserve(middleSPvec.size());
for (const Acts::InternalSpacePoint<external_spacepoint_t>* SP :
middleSPvec) {
for (auto SP : middleSPvec) {
deviceMiddleSPs.push_back({SP->x(), SP->y(), SP->z(), SP->radius(),
SP->varianceR(), SP->varianceZ()});
}

for (const Acts::InternalSpacePoint<external_spacepoint_t>* SP : topSPs) {
for (auto SP : topSPs) {
topSPvec.push_back(SP);
}
deviceTopSPs.reserve(topSPvec.size());
for (const Acts::InternalSpacePoint<external_spacepoint_t>* SP : topSPvec) {
for (auto SP : topSPvec) {
deviceTopSPs.push_back({SP->x(), SP->y(), SP->z(), SP->radius(),
SP->varianceR(), SP->varianceZ()});
}
Expand Down Expand Up @@ -149,4 +145,4 @@ Seedfinder<external_spacepoint_t>::createSeedsForGroup(
}
return outputVec;
}
} // namespace Acts::Sycl
} // namespace Acts::Sycl