Skip to content

Commit

Permalink
refactor!: Do not use geometry extent during seeding (#3688)
Browse files Browse the repository at this point in the history
In the seeding we only need the radius range of the space points (and
only when the range is computed by the code and not provided by the
user). As of now, when filling the grid we waste a lot of time to fill
the `Acts::Extent` object (for all the possible `Acts::BinningValue`
even if we don't need them all).

I have refactored the code so that we do not do this expensive
operation. The user will have to do it by them selves or provide a
validity search window via JO.

No changes expected from this PR
  • Loading branch information
CarloVarni authored Oct 4, 2024
1 parent 5c5d085 commit 81e129b
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#pragma once

#include "Acts/Geometry/Extent.hpp"
#include "Acts/Seeding/BinnedGroup.hpp"
#include "Acts/Seeding/SeedFinderConfig.hpp"
#include "Acts/Utilities/Grid.hpp"
Expand Down Expand Up @@ -121,7 +120,7 @@ class CylindricalSpacePointGridCreator {
const Acts::SeedFinderOptions& options,
Acts::CylindricalSpacePointGrid<external_spacepoint_t>& grid,
external_spacepoint_iterator_t spBegin,
external_spacepoint_iterator_t spEnd, Acts::Extent& rRangeSPExtent);
external_spacepoint_iterator_t spEnd);
};

} // namespace Acts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
const Acts::SeedFinderOptions& options,
Acts::CylindricalSpacePointGrid<external_spacepoint_t>& grid,
external_spacepoint_iterator_t spBegin,
external_spacepoint_iterator_t spEnd, Acts::Extent& rRangeSPExtent) {
external_spacepoint_iterator_t spEnd) {
if (!config.isInInternalUnits) {
throw std::runtime_error(
"SeedFinderConfig not in ACTS internal units in BinnedSPGroup");
Expand Down Expand Up @@ -176,9 +176,6 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid(
float spY = sp.y();
float spZ = sp.z();

// store x,y,z values in extent
rRangeSPExtent.extend({spX, spY, spZ});

// remove SPs according to experiment specific cuts
if (!config.spacePointSelector(sp)) {
continue;
Expand Down
30 changes: 18 additions & 12 deletions Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "Acts/Definitions/Algebra.hpp"
#include "Acts/EventData/Seed.hpp"
#include "Acts/EventData/SpacePointData.hpp"
#include "Acts/Geometry/Extent.hpp"
#include "Acts/Seeding/BinnedGroup.hpp"
#include "Acts/Seeding/SeedFilter.hpp"
#include "Acts/Utilities/BinningType.hpp"
Expand Down Expand Up @@ -243,16 +242,28 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute(
using value_type = typename decltype(spContainer)::SpacePointProxyType;
using seed_type = Acts::Seed<value_type>;

// extent used to store r range for middle spacepoint
Acts::Extent rRangeSPExtent;

Acts::CylindricalSpacePointGrid<value_type> grid =
Acts::CylindricalSpacePointGridCreator::createGrid<value_type>(
m_cfg.gridConfig, m_cfg.gridOptions);

Acts::CylindricalSpacePointGridCreator::fillGrid(
m_cfg.seedFinderConfig, m_cfg.seedFinderOptions, grid,
spContainer.begin(), spContainer.end(), rRangeSPExtent);
spContainer.begin(), spContainer.end());

// Compute radius Range
// we rely on the fact the grid is storing the proxies
// with a sorting in the radius
float minRange = std::numeric_limits<float>::max();
float maxRange = std::numeric_limits<float>::lowest();
for (const auto& coll : grid) {
if (coll.empty()) {
continue;
}
const auto* firstEl = coll.front();
const auto* lastEl = coll.back();
minRange = std::min(firstEl->radius(), minRange);
maxRange = std::max(lastEl->radius(), maxRange);
}

std::array<std::vector<std::size_t>, 2ul> navigation;
navigation[1ul] = m_cfg.seedFinderConfig.zBinsCustomLooping;
Expand All @@ -261,15 +272,10 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute(
std::move(grid), *m_bottomBinFinder, *m_topBinFinder,
std::move(navigation));

// safely clamp double to float
float up = Acts::clampValue<float>(
std::floor(rRangeSPExtent.max(Acts::BinningValue::binR) / 2) * 2);

/// variable middle SP radial region of interest
const Acts::Range1D<float> rMiddleSPRange(
std::floor(rRangeSPExtent.min(Acts::BinningValue::binR) / 2) * 2 +
m_cfg.seedFinderConfig.deltaRMiddleMinSPRange,
up - m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange);
minRange + m_cfg.seedFinderConfig.deltaRMiddleMinSPRange,
maxRange - m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange);

// run the seeding
static thread_local std::vector<seed_type> seeds;
Expand Down
29 changes: 18 additions & 11 deletions Examples/Algorithms/TrackFinding/src/SeedingAlgorithmHashing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "Acts/Definitions/Algebra.hpp"
#include "Acts/EventData/Seed.hpp"
#include "Acts/EventData/SpacePointData.hpp"
#include "Acts/Geometry/Extent.hpp"
#include "Acts/Plugins/Hashing/HashingAlgorithm.hpp"
#include "Acts/Plugins/Hashing/HashingTraining.hpp"
#include "Acts/Seeding/BinnedGroup.hpp"
Expand Down Expand Up @@ -269,15 +268,28 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithmHashing::execute(
Acts::SpacePointContainer<decltype(container), Acts::detail::RefHolder>
spContainer(spConfig, spOptions, container);

// extent used to store r range for middle spacepoint
Acts::Extent rRangeSPExtent;
// construct the seeding tools
Acts::CylindricalSpacePointGrid<value_type> grid =
Acts::CylindricalSpacePointGridCreator::createGrid<value_type>(
m_cfg.gridConfig, m_cfg.gridOptions);
Acts::CylindricalSpacePointGridCreator::fillGrid(
m_cfg.seedFinderConfig, m_cfg.seedFinderOptions, grid,
spContainer.begin(), spContainer.end(), rRangeSPExtent);
spContainer.begin(), spContainer.end());

// Compute radius Range
// we rely on the fact the grid is storing the proxies
// with a sorting in the radius
float minRange = std::numeric_limits<float>::max();
float maxRange = std::numeric_limits<float>::lowest();
for (const auto& coll : grid) {
if (coll.empty()) {
continue;
}
const auto* firstEl = coll.front();
const auto* lastEl = coll.back();
minRange = std::min(firstEl->radius(), minRange);
maxRange = std::max(lastEl->radius(), maxRange);
}

std::array<std::vector<std::size_t>, 2ul> navigation;
navigation[1ul] = m_cfg.seedFinderConfig.zBinsCustomLooping;
Expand All @@ -287,15 +299,10 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithmHashing::execute(
std::move(grid), *m_bottomBinFinder, *m_topBinFinder,
std::move(navigation));

// safely clamp double to float
float up = Acts::clampValue<float>(
std::floor(rRangeSPExtent.max(Acts::BinningValue::binR) / 2) * 2);

/// variable middle SP radial region of interest
const Acts::Range1D<float> rMiddleSPRange(
std::floor(rRangeSPExtent.min(Acts::BinningValue::binR) / 2) * 2 +
m_cfg.seedFinderConfig.deltaRMiddleMinSPRange,
up - m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange);
minRange + m_cfg.seedFinderConfig.deltaRMiddleMinSPRange,
maxRange - m_cfg.seedFinderConfig.deltaRMiddleMaxSPRange);

// this creates seeds of proxy, we need to convert it to seed of space
// points
Expand Down
7 changes: 1 addition & 6 deletions Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "Acts/Definitions/Units.hpp"
#include "Acts/EventData/Seed.hpp"
#include "Acts/EventData/SpacePointContainer.hpp"
#include "Acts/Geometry/Extent.hpp"
#include "Acts/Seeding/BinnedGroup.hpp"
#include "Acts/Seeding/SeedFilter.hpp"
#include "Acts/Seeding/SeedFilterConfig.hpp"
Expand Down Expand Up @@ -173,9 +172,6 @@ int main(int argc, char** argv) {

int numPhiNeighbors = 1;

// extent used to store r range for middle spacepoint
Acts::Extent rRangeSPExtent;

config.useVariableMiddleSPRange = false;
const Acts::Range1D<float> rMiddleSPRange;

Expand Down Expand Up @@ -213,8 +209,7 @@ int main(int argc, char** argv) {
Acts::CylindricalSpacePointGridCreator::createGrid<value_type>(gridConf,
gridOpts);
Acts::CylindricalSpacePointGridCreator::fillGrid(
config, options, grid, spContainer.begin(), spContainer.end(),
rRangeSPExtent);
config, options, grid, spContainer.begin(), spContainer.end());

auto spGroup = Acts::CylindricalBinnedGroup<value_type>(
std::move(grid), *bottomBinFinder, *topBinFinder);
Expand Down

0 comments on commit 81e129b

Please sign in to comment.