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

feat!: Generic grid bin finder #2838

Merged
merged 96 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 92 commits
Commits
Show all changes
96 commits
Select commit Hold shift + click to select a range
9552ebc
First implementation
Nov 21, 2023
ae0724c
changes
Nov 22, 2023
f9d16c0
no future
Nov 23, 2023
501f16c
minor
Nov 23, 2023
6aa0a62
format
Nov 23, 2023
4326ed6
first round of tests
Nov 24, 2023
558efd5
sub iteration
Dec 12, 2023
cb8de3f
format
Dec 12, 2023
b9cf4cc
changes
Dec 12, 2023
70c7feb
format
Dec 12, 2023
a77b90b
format
Dec 12, 2023
02bf47b
format
Dec 12, 2023
b53dedd
test 1 for cuda
Dec 12, 2023
b8793b1
local, not global
Dec 12, 2023
bc77b3e
agains cuda
Dec 12, 2023
0f0b59d
and again format
Dec 12, 2023
bf60dfd
more tests for global iterator
Dec 12, 2023
e074e16
more tests and some bug fixing
Dec 12, 2023
1c55a27
oopsie
Dec 12, 2023
9179279
even more tests
Dec 13, 2023
a35da6d
remove line
Dec 13, 2023
773d11c
format
Dec 13, 2023
f7fdcb5
first round of comments
Dec 13, 2023
d5016b8
format
Dec 13, 2023
ec66bfa
test for checking iterations
Dec 13, 2023
9ea63ab
clang tidy
Dec 13, 2023
4772218
remove property
Dec 15, 2023
bc0aa70
First implementation
Nov 21, 2023
167756d
changes
Nov 22, 2023
b12d268
no future
Nov 23, 2023
cc20c8d
minor
Nov 23, 2023
5285668
format
Nov 23, 2023
7c9d4e4
first round of tests
Nov 24, 2023
5d7e2ab
sub iteration
Dec 12, 2023
e0d0fe7
format
Dec 12, 2023
309a0ef
changes
Dec 12, 2023
ab158e1
format
Dec 12, 2023
52b90cd
format
Dec 12, 2023
118db39
format
Dec 12, 2023
592183b
test 1 for cuda
Dec 12, 2023
c2ad48b
local, not global
Dec 12, 2023
57f8876
agains cuda
Dec 12, 2023
b1abea1
and again format
Dec 12, 2023
3e9599c
more tests for global iterator
Dec 12, 2023
0c70d03
more tests and some bug fixing
Dec 12, 2023
f759e1e
oopsie
Dec 12, 2023
63917d9
even more tests
Dec 13, 2023
3d31631
remove line
Dec 13, 2023
1bba1e9
format
Dec 13, 2023
bfe8e61
first round of comments
Dec 13, 2023
86af08d
format
Dec 13, 2023
d627255
test for checking iterations
Dec 13, 2023
bd7bd05
clang tidy
Dec 13, 2023
ec2fe30
move and rename
Dec 15, 2023
bcf7ec1
first working version
Dec 15, 2023
edc349b
changes
Dec 15, 2023
1da0f92
sync
Dec 15, 2023
e39777f
tests
Dec 16, 2023
73a5600
changes
Dec 16, 2023
cdce928
changes
Dec 16, 2023
c39a265
sync
Dec 16, 2023
65d249a
formta
Dec 16, 2023
ee1e504
Merge branch 'main' into removeSkipProperty
paulgessinger Dec 17, 2023
1c5cc86
cuda
Dec 18, 2023
4f64437
new methods and tests
Dec 18, 2023
601339e
Add comments
Dec 18, 2023
afa5acf
format
Dec 18, 2023
a325bc2
spelling
Dec 18, 2023
8b7374f
add protections
Dec 18, 2023
59eb33f
Core
Dec 18, 2023
c0f3253
first input check
Dec 18, 2023
ee18f0f
use asserts
Dec 18, 2023
072d184
typo
Dec 18, 2023
5df2442
bug
Dec 18, 2023
30565d7
to be reverted change
Dec 18, 2023
41a10bd
Update builds.yml
CarloVarni Dec 18, 2023
f0728e0
Protection against empty zbin neighbours in example seeding alg
CarloVarni Dec 18, 2023
04c9ba2
Update SeedingAlgorithm.cpp
CarloVarni Dec 18, 2023
1adf1a8
test
Dec 19, 2023
a3a772b
set default num phi neighbours to 1
Dec 19, 2023
dfe672f
update comment
Dec 19, 2023
3800426
format, license and spelling
Dec 19, 2023
63669c1
test with empty inputs
Dec 19, 2023
c5078f3
Merge remote-tracking branch 'upstream/main' into GenericBinFinder
Dec 19, 2023
623b3de
clang tidy
Dec 19, 2023
66f8342
test constructor
Dec 19, 2023
1b16b19
Merge remote-tracking branch 'upstream/main' into GenericBinFinder
Dec 21, 2023
31de7db
Merge remote-tracking branch 'origin/AddCommentsGridIterator' into Ge…
Dec 21, 2023
a079084
add other changes
Dec 21, 2023
b9e94c6
sync
Jan 9, 2024
3d474f7
copyright and typos
Jan 9, 2024
5e39602
test for boundary types
Jan 9, 2024
9e408e0
sync
Jan 10, 2024
86b25ea
fix test and format
Jan 10, 2024
cbe587c
sync
Jan 15, 2024
62ebb3b
Merge branch 'main' into GenericBinFinder
kodiakhq[bot] Jan 16, 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
51 changes: 0 additions & 51 deletions Core/include/Acts/Seeding/BinFinder.hpp

This file was deleted.

29 changes: 0 additions & 29 deletions Core/include/Acts/Seeding/BinFinder.ipp

This file was deleted.

40 changes: 15 additions & 25 deletions Core/include/Acts/Seeding/BinnedSPGroup.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2023 CERN for the benefit of the Acts project
// Copyright (C) 2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
Expand All @@ -9,11 +9,11 @@
#pragma once

#include "Acts/Geometry/Extent.hpp"
#include "Acts/Seeding/BinFinder.hpp"
#include "Acts/Seeding/InternalSeed.hpp"
#include "Acts/Seeding/Seed.hpp"
#include "Acts/Seeding/SeedFinderConfig.hpp"
#include "Acts/Seeding/SpacePointGrid.hpp"
#include "Acts/Utilities/GridBinFinder.hpp"
#include "Acts/Utilities/GridIterator.hpp"
#include "Acts/Utilities/Holders.hpp"

Expand All @@ -27,7 +27,7 @@ template <typename external_spacepoint_t>
class BinnedSPGroup;

/// @c BinnedSPGroupIterator Allows to iterate over all groups of bins
/// a provided BinFinder can generate for each bin of a provided SPGrid
/// a provided GridBinFinder can generate for each bin of a provided SPGrid

/// SpacePointGrid is a very specific structure.
/// We know it is 2D and what it contains
Expand Down Expand Up @@ -79,7 +79,7 @@ class BinnedSPGroupIterator {
};

/// @c BinnedSPGroup Provides access to begin and end BinnedSPGroupIterator
/// for given BinFinders and SpacePointGrid.
/// for given GridBinFinders and SpacePointGrid.
/// Fulfills the range_expression interface.
template <typename external_spacepoint_t>
class BinnedSPGroup {
Expand All @@ -93,16 +93,14 @@ class BinnedSPGroup {
BinnedSPGroup() = delete;

template <typename spacepoint_iterator_t, typename callable_t>
BinnedSPGroup(
spacepoint_iterator_t spBegin, spacepoint_iterator_t spEnd,
callable_t&& toGlobal,
std::shared_ptr<const Acts::BinFinder<external_spacepoint_t>>
botBinFinder,
std::shared_ptr<const Acts::BinFinder<external_spacepoint_t>> tBinFinder,
std::unique_ptr<SpacePointGrid<external_spacepoint_t>> grid,
Acts::Extent& rRangeSPExtent,
const SeedFinderConfig<external_spacepoint_t>& _config,
const SeedFinderOptions& _options);
BinnedSPGroup(spacepoint_iterator_t spBegin, spacepoint_iterator_t spEnd,
callable_t&& toGlobal,
std::shared_ptr<const Acts::GridBinFinder<2ul>> botBinFinder,
std::shared_ptr<const Acts::GridBinFinder<2ul>> tBinFinder,
std::unique_ptr<SpacePointGrid<external_spacepoint_t>> grid,
Acts::Extent& rRangeSPExtent,
const SeedFinderConfig<external_spacepoint_t>& _config,
const SeedFinderOptions& _options);

BinnedSPGroup(const BinnedSPGroup&) = delete;
BinnedSPGroup& operator=(const BinnedSPGroup&) = delete;
Expand All @@ -121,25 +119,17 @@ class BinnedSPGroup {
return *m_grid.get();
}

std::size_t skipZMiddleBin() {
return m_skipZMiddleBin;
}

private:
// grid with ownership of all InternalSpacePoint
std::unique_ptr<Acts::SpacePointGrid<external_spacepoint_t>> m_grid{nullptr};

// BinFinder must return std::vector<Acts::Seeding::Bin> with content of
// GridBinFinder must return std::vector<Acts::Seeding::Bin> with content of
// each bin sorted in r (ascending)
std::shared_ptr<const BinFinder<external_spacepoint_t>> m_topBinFinder{
nullptr};
std::shared_ptr<const BinFinder<external_spacepoint_t>> m_bottomBinFinder{
nullptr};
std::shared_ptr<const Acts::GridBinFinder<2ul>> m_topBinFinder{nullptr};
std::shared_ptr<const Acts::GridBinFinder<2ul>> m_bottomBinFinder{nullptr};

// Order of z bins to loop over when searching for SPs
std::array<std::vector<std::size_t>, 2> m_bins{};
// Number of Z bins to skip the search for middle SP
std::size_t m_skipZMiddleBin{0ul};
};

} // namespace Acts
Expand Down
40 changes: 16 additions & 24 deletions Core/include/Acts/Seeding/BinnedSPGroup.ipp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2023 CERN for the benefit of the Acts project
// Copyright (C) 2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
Expand Down Expand Up @@ -55,13 +55,10 @@ Acts::BinnedSPGroupIterator<external_spacepoint_t>::operator*() const {
m_group->m_grid->globalBinFromLocalBins(localPosition);

boost::container::small_vector<std::size_t, 9> bottoms =
m_group->m_bottomBinFinder->findBins(localPosition[INDEX::PHI],
localPosition[INDEX::Z],
m_group->m_grid.get());
m_group->m_bottomBinFinder->findBins(localPosition,
*m_group->m_grid.get());
boost::container::small_vector<std::size_t, 9> tops =
m_group->m_topBinFinder->findBins(localPosition[INDEX::PHI],
localPosition[INDEX::Z],
m_group->m_grid.get());
m_group->m_topBinFinder->findBins(localPosition, *m_group->m_grid.get());

// GCC12+ in Release throws an overread warning here due to the move.
// This is from inside boost code, so best we can do is to suppress it.
Expand Down Expand Up @@ -95,12 +92,15 @@ template <typename spacepoint_iterator_t, typename callable_t>
Acts::BinnedSPGroup<external_spacepoint_t>::BinnedSPGroup(
spacepoint_iterator_t spBegin, spacepoint_iterator_t spEnd,
callable_t&& toGlobal,
std::shared_ptr<const Acts::BinFinder<external_spacepoint_t>> botBinFinder,
std::shared_ptr<const Acts::BinFinder<external_spacepoint_t>> tBinFinder,
std::shared_ptr<const Acts::GridBinFinder<2ul>> botBinFinder,
std::shared_ptr<const Acts::GridBinFinder<2ul>> tBinFinder,
std::unique_ptr<SpacePointGrid<external_spacepoint_t>> grid,
Acts::Extent& rRangeSPExtent,
const SeedFinderConfig<external_spacepoint_t>& config,
const SeedFinderOptions& options) {
const SeedFinderOptions& options)
: m_grid(std::move(grid)),
m_topBinFinder(std::move(tBinFinder)),
m_bottomBinFinder(std::move(botBinFinder)) {
if (!config.isInInternalUnits) {
throw std::runtime_error(
"SeedFinderConfig not in ACTS internal units in BinnedSPGroup");
Expand Down Expand Up @@ -171,20 +171,20 @@ Acts::BinnedSPGroup<external_spacepoint_t>::BinnedSPGroup(
// fill rbins into grid
Acts::Vector2 spLocation(isp->phi(), isp->z());
std::vector<std::unique_ptr<InternalSpacePoint<external_spacepoint_t>>>&
rbin = grid->atPosition(spLocation);
rbin = m_grid->atPosition(spLocation);
rbin.push_back(std::move(isp));

// keep track of the bins we modify so that we can later sort the SPs in
// those bins only
if (rbin.size() > 1) {
rBinsIndex.insert(grid->globalBinFromPosition(spLocation));
rBinsIndex.insert(m_grid->globalBinFromPosition(spLocation));
}
}

// sort SPs in R for each filled (z, phi) bin
for (auto& binIndex : rBinsIndex) {
std::vector<std::unique_ptr<InternalSpacePoint<external_spacepoint_t>>>&
rbin = grid->atPosition(binIndex);
rbin = m_grid->atPosition(binIndex);
std::sort(
rbin.begin(), rbin.end(),
[](std::unique_ptr<InternalSpacePoint<external_spacepoint_t>>& a,
Expand All @@ -193,26 +193,18 @@ Acts::BinnedSPGroup<external_spacepoint_t>::BinnedSPGroup(
});
}

m_grid = std::move(grid);
m_bottomBinFinder = botBinFinder;
m_topBinFinder = tBinFinder;

m_skipZMiddleBin = config.skipZMiddleBinSearch;

// phi axis
m_bins[INDEX::PHI].resize(m_grid->numLocalBins()[0]);
std::iota(m_bins[INDEX::PHI].begin(), m_bins[INDEX::PHI].end(), 1ul);

// z axis
if (config.zBinsCustomLooping.empty()) {
std::size_t nZbins = m_grid->numLocalBins()[1] - m_skipZMiddleBin;
std::size_t nZbins = m_grid->numLocalBins()[INDEX::Z];
m_bins[INDEX::Z] = std::vector<std::size_t>(nZbins);
std::iota(m_bins[INDEX::Z].begin(), m_bins[INDEX::Z].end(),
1ul + m_skipZMiddleBin);
std::iota(m_bins[INDEX::Z].begin(), m_bins[INDEX::Z].end(), 1ul);
} else {
m_bins[INDEX::Z] = std::vector<std::size_t>(
config.zBinsCustomLooping.begin() + m_skipZMiddleBin,
config.zBinsCustomLooping.end());
m_bins[INDEX::Z] = config.zBinsCustomLooping;
}
}

Expand Down
5 changes: 0 additions & 5 deletions Core/include/Acts/Seeding/SeedFinderConfig.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ struct SeedFinderConfig {
/// Vector containing the z-bin edges for non equidistant binning in z
std::vector<float> zBinEdges;

/// Number of z bins to skip during the search for middle space-points. This
/// is useful for removing the outermost bins and speeding up seeding. Should
/// be used in conjunction with zBinsCustomLooping (skipZMiddleBinSearch
/// determines the first N bins in zBinsCustomLooping to be avoided).
std::size_t skipZMiddleBinSearch = 0;
/// Order of z bins to loop over when searching for SPs
std::vector<std::size_t> zBinsCustomLooping = {};

Expand Down
114 changes: 114 additions & 0 deletions Core/include/Acts/Utilities/GridBinFinder.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// This file is part of the Acts project.
//
// Copyright (C) 2024 CERN for the benefit of the Acts project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

#pragma once

#include "Acts/Seeding/SpacePointGrid.hpp"
CarloVarni marked this conversation as resolved.
Show resolved Hide resolved
#include "Acts/Utilities/Holders.hpp"
#include "Acts/Utilities/detail/grid_helper.hpp"

#include <variant>
#include <vector>

#include <boost/container/small_vector.hpp>

namespace Acts {

/// @class BinFinder
/// @tparam DIM Dimension of the Grid on which the GridBinFinder will be used
///
/// The BinFinder is used by the ISPGroupSelector. It can be
/// used to find both bins that could be bottom bins as well as bins that could
/// be top bins, which are assumed to be the same bins. Does not take
/// interaction region into account to limit z-bins.
template <std::size_t DIM>
class GridBinFinder {
public:
/// @brief Constructor
/// @tparam args ... Input parameters provided by the user
///
/// @param [in] vals The input parameters that define how many neighbours we need to find
///
/// @pre The provided paramers must be of time 'int' or 'std::vector<std::pair<int, int>>'
/// no other type is allowed. The order of these parameters must correspond to
/// the same ordering of the axes in the grid
template <typename... args>
GridBinFinder(args&&... vals);

/// @brief Retrieve the neighbouring bins given a local position in the grid
///
/// Return all bins that could contain space points that can be used with the
/// space points in the bin with the provided indices to create seeds.
///
/// @tparam stored_t The type of elements stored in the Grid
/// @tpatam Axes ... The type of the axes of the grid
///
/// @param [in] locPosition The N-dimentional local position in the grid
/// @param [in] grid The grid
/// @output The list of neighbouring bins
///
/// @pre The provided local position must be a valid local bins configuration in the grid
template <typename stored_t, class... Axes>
boost::container::small_vector<std::size_t, Acts::detail::ipow(3, DIM)>
findBins(const std::array<std::size_t, DIM>& locPosition,
const Acts::Grid<stored_t, Axes...>& grid) const;

private:
/// @brief Store the values provided by the user for each axis in the grid
/// @tparam first_value_t Type of the first value
/// @tparam vals ... values of the remaining values
///
/// @param [in] fv The first value in the list
/// @param [in] others The remaining values in the list
///
/// @pre both first_value_t and vals ... can be only int or std::vector<std::pair<int, int>>
/// In the second case, the number of entries of the vector of pairs MUST be
/// equal to the number of bins in that specific axis. Empty vectors are also
/// allowed but in this case the value will be replaced with a 1 (integer),
/// thus instructing the code to look for neighbours in the range {-1 ,1}
template <typename first_value_t, typename... vals>
void storeValue(first_value_t&& fv, vals&&... others);

/// @brief Get the instructions for retrieving the neighbouring bins given a local position
///
/// @param [in] locPosition The requested local position
/// @return the instructions for retrieving the neighbouring bins for this local position
///
/// @pre The local position must be a valid local bins configuration for the grid
std::array<std::pair<int, int>, DIM> getSizePerAxis(
const std::array<std::size_t, DIM>& locPosition) const;

/// @brief Check the GridBinFinder configuration is compatible with the grid
/// by checking the values of m_values against the axes of the grid
/// This function is called only in debug mode
///
/// @tparam stored_t The type of elements stored in the Grid
/// @tpatam Axes ... The type of the axes of the grid
///
/// @param [in] grid The Grid
/// @return If the GridBinFinder is compatible with the grid
template <typename stored_t, class... Axes>
bool isGridCompatible(const Acts::Grid<stored_t, Axes...>& grid) const;

private:
using stored_values_t = std::variant<int, std::vector<std::pair<int, int>>>;
CarloVarni marked this conversation as resolved.
Show resolved Hide resolved
/// @brief the instructions for retrieving the nieghbouring bins for each given axis in the grid
/// These values are provided by the user and can be either ints or a vector of
/// pair of ints. In the first case, the neighbours will be +/- bins from the
/// given local bin In the second case, the user defines how many bins in both
/// directions should be provided
///
/// @pre The list of entries of the vector of pairs MUST be equal to the number of bins in that specific
/// axis. Empty vectors are also allowed but in this case the value will be
/// replaced with a 1 (integer), thus instructing the code to look for
/// neighbours in the range {-1 ,1}
std::array<stored_values_t, DIM> m_values{};
};

} // namespace Acts
#include "Acts/Utilities/GridBinFinder.ipp"
Loading
Loading