Skip to content

Commit

Permalink
refactor!: use c++20 concept to define insert function for different …
Browse files Browse the repository at this point in the history
…types of seed collections (#3367)

Taking from @<!---->CouthuresJeremy 's PR I'm here proposing a possible alternative that would not even be a breaking change (unless I'm missing something). This will use c++20 concepts to define different versions of an `insert` function that will add a seed to the output collection.

This will work with 'back_insert_operator' (as we currently do) as well as collection that support `push_back` and/or `insert`

blocked by

- #3345
- #3377

edit: Now this is breaking since we do not support back_insert_operator option anymore
  • Loading branch information
CarloVarni authored Jul 16, 2024
1 parent b4c888d commit 88c1223
Show file tree
Hide file tree
Showing 11 changed files with 177 additions and 89 deletions.
24 changes: 11 additions & 13 deletions Core/include/Acts/Seeding/SeedFilter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ struct SeedFilterState {
/// Filter seeds at various stages with the currently
/// available information.
template <typename external_spacepoint_t>
class SeedFilter {
class SeedFilter final {
public:
SeedFilter(SeedFilterConfig config,
IExperimentCuts<external_spacepoint_t>* expCuts = nullptr);

SeedFilter() = delete;
virtual ~SeedFilter() = default;
~SeedFilter() = default;

/// Create InternalSeeds for the all seeds with the same bottom and middle
/// space point and discard all others.
Expand All @@ -61,7 +61,7 @@ class SeedFilter {
/// @param impactParametersVec vector containing the impact parameters
/// @param seedFilterState holds quantities used in seed filter
/// @param candidates_collector container for the seed candidates
virtual void filterSeeds_2SpFixed(
void filterSeeds_2SpFixed(
Acts::SpacePointData& spacePointData,
const InternalSpacePoint<external_spacepoint_t>& bottomSP,
const InternalSpacePoint<external_spacepoint_t>& middleSP,
Expand All @@ -77,30 +77,28 @@ class SeedFilter {
/// @param spacePointData Auxiliary variables used by the seeding
/// @param candidates_collector collection of seed candidates
/// @param numQualitySeeds number of high quality seeds in seed confirmation
/// @param outIt Output iterator for the seeds
/// @param outputCollection Output container for the seeds
/// for all seeds with the same middle space point
virtual void filterSeeds_1SpFixed(
template <typename collection_t>
void filterSeeds_1SpFixed(
Acts::SpacePointData& spacePointData,
CandidatesForMiddleSp<const InternalSpacePoint<external_spacepoint_t>>&
candidates_collector,
const std::size_t numQualitySeeds,
std::back_insert_iterator<std::vector<Seed<external_spacepoint_t>>> outIt)
const;
const std::size_t numQualitySeeds, collection_t& outputCollection) const;

/// Filter seeds once all seeds for one middle space point have been created
/// @param spacePointData Auxiliary variables used by the seeding
/// @param candidates collection of seed candidates
/// @param numQualitySeeds number of high quality seeds in seed confirmation
/// @param outIt Output iterator for the seeds
/// @param outputCollection Output container for the seeds
/// for all seeds with the same middle space point
virtual void filterSeeds_1SpFixed(
template <typename collection_t>
void filterSeeds_1SpFixed(
Acts::SpacePointData& spacePointData,
std::vector<typename CandidatesForMiddleSp<
const InternalSpacePoint<external_spacepoint_t>>::value_type>&
candidates,
const std::size_t numQualitySeeds,
std::back_insert_iterator<std::vector<Seed<external_spacepoint_t>>> outIt)
const;
const std::size_t numQualitySeeds, collection_t& outputCollection) const;

const SeedFilterConfig getSeedFilterConfig() const { return m_cfg; }
const IExperimentCuts<external_spacepoint_t>* getExperimentCuts() const {
Expand Down
21 changes: 12 additions & 9 deletions Core/include/Acts/Seeding/SeedFilter.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
// 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/.

#include "Acts/Seeding/detail/UtilityFunctions.hpp"

#include <algorithm>
#include <numeric>
#include <utility>
Expand Down Expand Up @@ -245,30 +247,28 @@ void SeedFilter<external_spacepoint_t>::filterSeeds_2SpFixed(
// after creating all seeds with a common middle space point, filter again

template <typename external_spacepoint_t>
template <typename collection_t>
void SeedFilter<external_spacepoint_t>::filterSeeds_1SpFixed(
Acts::SpacePointData& spacePointData,
CandidatesForMiddleSp<const InternalSpacePoint<external_spacepoint_t>>&
candidates_collector,
const std::size_t numQualitySeeds,
std::back_insert_iterator<std::vector<Seed<external_spacepoint_t>>> outIt)
const {
const std::size_t numQualitySeeds, collection_t& outputCollection) const {
// retrieve all candidates
// this collection is already sorted
// higher weights first
auto extended_collection = candidates_collector.storage();
filterSeeds_1SpFixed(spacePointData, extended_collection, numQualitySeeds,
outIt);
outputCollection);
}

template <typename external_spacepoint_t>
template <typename collection_t>
void SeedFilter<external_spacepoint_t>::filterSeeds_1SpFixed(
Acts::SpacePointData& spacePointData,
std::vector<typename CandidatesForMiddleSp<
const InternalSpacePoint<external_spacepoint_t>>::value_type>&
candidates,
const std::size_t numQualitySeeds,
std::back_insert_iterator<std::vector<Seed<external_spacepoint_t>>> outIt)
const {
const std::size_t numQualitySeeds, collection_t& outputCollection) const {
if (m_experimentCuts != nullptr) {
candidates = m_experimentCuts->cutPerMiddleSP(std::move(candidates));
}
Expand Down Expand Up @@ -307,8 +307,11 @@ void SeedFilter<external_spacepoint_t>::filterSeeds_1SpFixed(
spacePointData.setQuality(medium->index(), bestSeedQuality);
spacePointData.setQuality(top->index(), bestSeedQuality);

outIt = Seed<external_spacepoint_t>{bottom->sp(), medium->sp(), top->sp(),
zOrigin, bestSeedQuality};
Acts::detail::pushBackOrInsertAtEnd(
outputCollection,
Seed<external_spacepoint_t>{bottom->sp(), medium->sp(), top->sp(),
zOrigin, bestSeedQuality});

++numTotalSeeds;
}
}
Expand Down
43 changes: 9 additions & 34 deletions Core/include/Acts/Seeding/SeedFinder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,46 +92,21 @@ class SeedFinder {
/// @param options frequently changing configuration (like beam position)
/// @param state State object that holds memory used
/// @param grid The grid with space points
/// @param outIt Output iterator for the seeds in the group
/// @param outputCollection Output container for the seeds in the group
/// @param bottomSPs group of space points to be used as innermost SP in a seed.
/// @param middleSPs group of space points to be used as middle SP in a seed.
/// @param topSPs group of space points to be used as outermost SP in a seed.
/// @param rMiddleSPRange range object containing the minimum and maximum r for middle SP for a certain z bin.
/// @note Ranges must return pointers.
/// @note Ranges must be separate objects for each parallel call.
template <template <typename...> typename container_t, typename sp_range_t>
void createSeedsForGroup(
const Acts::SeedFinderOptions& options, SeedingState& state,
const grid_t& grid,
std::back_insert_iterator<container_t<Seed<external_spacepoint_t>>> outIt,
const sp_range_t& bottomSPs, const std::size_t middleSPs,
const sp_range_t& topSPs,
const Acts::Range1D<float>& rMiddleSPRange) const;

/// @brief Compatibility method for the new-style seed finding API.
///
/// This method models the old-style seeding API where we only need a
/// container for the bottom, middle, and top space points. Also, the results
/// are returned by value instead of inserted into an inserter.
///
/// @note This method is a very simply wrapper around the more modern API.
/// @warning The performance of the seeding code is far greater if the new
/// API is used, and this is recommended for all new uses which do not
/// require backwards-compatibility.
///
/// @tparam sp_range_t container type for the seed point collections.
/// @param options frequently changing configuration (like beam position)
/// @param grid The grid with space points
/// @param bottomSPs group of space points to be used as innermost SP in a
/// seed.
/// @param middleSPs group of space points to be used as middle SP in a seed.
/// @param topSPs group of space points to be used as outermost SP in a seed.
/// @returns a vector of seeds.
template <typename sp_range_t>
std::vector<Seed<external_spacepoint_t>> createSeedsForGroup(
const Acts::SeedFinderOptions& options, const grid_t& grid,
const sp_range_t& bottomSPs, const std::size_t middleSPs,
const sp_range_t& topSPs) const;
template <typename container_t, typename sp_range_t>
void createSeedsForGroup(const Acts::SeedFinderOptions& options,
SeedingState& state, const grid_t& grid,
container_t& outputCollection,
const sp_range_t& bottomSPs,
const std::size_t middleSPs,
const sp_range_t& topSPs,
const Acts::Range1D<float>& rMiddleSPRange) const;

private:
/// Iterates over dublets and tests the compatibility between them by applying
Expand Down
23 changes: 3 additions & 20 deletions Core/include/Acts/Seeding/SeedFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,10 @@ SeedFinder<external_spacepoint_t, grid_t, platform_t>::SeedFinder(
}

template <typename external_spacepoint_t, typename grid_t, typename platform_t>
template <template <typename...> typename container_t, typename sp_range_t>
template <typename container_t, typename sp_range_t>
void SeedFinder<external_spacepoint_t, grid_t, platform_t>::createSeedsForGroup(
const Acts::SeedFinderOptions& options, SeedingState& state,
const grid_t& grid,
std::back_insert_iterator<container_t<Seed<external_spacepoint_t>>> outIt,
const grid_t& grid, container_t& outputCollection,
const sp_range_t& bottomSPsIdx, const std::size_t middleSPsIdx,
const sp_range_t& topSPsIdx,
const Acts::Range1D<float>& rMiddleSPRange) const {
Expand Down Expand Up @@ -201,7 +200,7 @@ void SeedFinder<external_spacepoint_t, grid_t, platform_t>::createSeedsForGroup(

m_config.seedFilter->filterSeeds_1SpFixed(
state.spacePointData, state.candidates_collector,
seedFilterState.numQualitySeeds, outIt);
seedFilterState.numQualitySeeds, outputCollection);

} // loop on mediums
}
Expand Down Expand Up @@ -831,20 +830,4 @@ SeedFinder<external_spacepoint_t, grid_t, platform_t>::filterCandidates(
} // loop on bottoms
}

template <typename external_spacepoint_t, typename grid_t, typename platform_t>
template <typename sp_range_t>
std::vector<Seed<external_spacepoint_t>>
SeedFinder<external_spacepoint_t, grid_t, platform_t>::createSeedsForGroup(
const Acts::SeedFinderOptions& options, const grid_t& grid,
const sp_range_t& bottomSPs, const std::size_t middleSPs,
const sp_range_t& topSPs) const {
SeedingState state;
const Acts::Range1D<float> rMiddleSPRange;
std::vector<Seed<external_spacepoint_t>> ret;

createSeedsForGroup(options, state, grid, std::back_inserter(ret), bottomSPs,
middleSPs, topSPs, rMiddleSPRange);

return ret;
}
} // namespace Acts
2 changes: 1 addition & 1 deletion Core/include/Acts/Seeding/SeedFinderOrthogonal.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ void SeedFinderOrthogonal<external_spacepoint_t>::processFromMiddleSP(
(!bottom_hl_v.empty() && !top_hl_v.empty())) {
m_config.seedFilter->filterSeeds_1SpFixed(
spacePointData, candidates_collector, seedFilterState.numQualitySeeds,
std::back_inserter(out_cont));
out_cont);
}
}

Expand Down
48 changes: 48 additions & 0 deletions Core/include/Acts/Seeding/detail/UtilityFunctions.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// 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 <ranges>

namespace Acts::detail {

// Define some concepts
template <typename external_t>
concept isCollectionThatSupportsPushBack =
std::ranges::range<external_t> && requires {
typename external_t::value_type;
} && requires(external_t coll, typename external_t::value_type val) {
coll.push_back(val);
};

template <typename external_t>
concept isCollectionThatSupportsInsert =
std::ranges::range<external_t> && requires {
typename external_t::value_type;
} && requires(external_t coll, typename external_t::value_type val) {
coll.insert(std::ranges::end(coll), val);
};

// Define some functions
template <typename value_t>
void pushBackOrInsertAtEnd(
Acts::detail::isCollectionThatSupportsPushBack auto& storage,
value_t&& value) {
storage.push_back(std::forward<value_t>(value));
}

template <std::ranges::range storage_t, typename value_t>
requires(!Acts::detail::isCollectionThatSupportsPushBack<storage_t> &&
Acts::detail::isCollectionThatSupportsInsert<
storage_t>) void pushBackOrInsertAtEnd(storage_t& storage,
value_t&& value) {
storage.insert(std::ranges::end(storage), std::forward<value_t>(value));
}

} // namespace Acts::detail
6 changes: 3 additions & 3 deletions Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute(
}

for (const auto [bottom, middle, top] : spacePointsGrouping) {
m_seedFinder.createSeedsForGroup(
m_cfg.seedFinderOptions, state, spacePointsGrouping.grid(),
std::back_inserter(seeds), bottom, middle, top, rMiddleSPRange);
m_seedFinder.createSeedsForGroup(m_cfg.seedFinderOptions, state,
spacePointsGrouping.grid(), seeds, bottom,
middle, top, rMiddleSPRange);
}

ACTS_DEBUG("Created " << seeds.size() << " track seeds from "
Expand Down
1 change: 1 addition & 0 deletions Tests/UnitTests/Core/Seeding/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ target_link_libraries(ActsUnitTestSeedFinder PRIVATE ActsCore Boost::boost)
add_unittest(EstimateTrackParamsFromSeed EstimateTrackParamsFromSeedTest.cpp)
add_unittest(BinnedGroupTest BinnedGroupTest.cpp)
add_unittest(HoughTransformTest HoughTransformTest.cpp)
add_unittest(UtilityFunctions UtilityFunctionsTests.cpp)
4 changes: 2 additions & 2 deletions Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ int main(int argc, char** argv) {
auto start = std::chrono::system_clock::now();
for (auto [bottom, middle, top] : spGroup) {
auto& v = seedVector.emplace_back();
a.createSeedsForGroup(options, state, spGroup.grid(), std::back_inserter(v),
bottom, middle, top, rMiddleSPRange);
a.createSeedsForGroup(options, state, spGroup.grid(), v, bottom, middle,
top, rMiddleSPRange);
}
auto end = std::chrono::system_clock::now();
std::chrono::duration<double> elapsed_seconds = end - start;
Expand Down
85 changes: 85 additions & 0 deletions Tests/UnitTests/Core/Seeding/UtilityFunctionsTests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// 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/.

#include <boost/test/unit_test.hpp>

#include "Acts/Seeding/detail/UtilityFunctions.hpp"

#include <iterator>
#include <list>
#include <set>
#include <string>
#include <unordered_set>
#include <vector>

namespace Acts::Test {

BOOST_AUTO_TEST_CASE(pushBackOrInsertAtEnd_vector) {
std::vector<std::size_t> coll;
Acts::detail::pushBackOrInsertAtEnd(coll, 2ul);
BOOST_CHECK(coll.size() == 1ul);
Acts::detail::pushBackOrInsertAtEnd(coll, 5ul);
BOOST_CHECK(coll.size() == 2ul);
std::size_t val = 1ul;
Acts::detail::pushBackOrInsertAtEnd(coll, val);
BOOST_CHECK(coll.size() == 3ul);

BOOST_CHECK_EQUAL(coll[0], 2ul);
BOOST_CHECK_EQUAL(coll[1], 5ul);
BOOST_CHECK_EQUAL(coll[2], 1ul);
}

BOOST_AUTO_TEST_CASE(pushBackOrInsertAtEnd_list) {
std::list<std::size_t> coll;
Acts::detail::pushBackOrInsertAtEnd(coll, 2ul);
BOOST_CHECK(coll.size() == 1ul);
Acts::detail::pushBackOrInsertAtEnd(coll, 5ul);
BOOST_CHECK(coll.size() == 2ul);
std::size_t val = 1ul;
Acts::detail::pushBackOrInsertAtEnd(coll, val);
BOOST_CHECK(coll.size() == 3ul);

BOOST_CHECK_EQUAL(coll.front(), 2ul);
coll.pop_front();
BOOST_CHECK_EQUAL(coll.front(), 5ul);
coll.pop_front();
BOOST_CHECK_EQUAL(coll.front(), 1ul);
coll.pop_front();
}

BOOST_AUTO_TEST_CASE(pushBackOrInsertAtEnd_set) {
std::set<std::size_t> coll;
Acts::detail::pushBackOrInsertAtEnd(coll, 2ul);
BOOST_CHECK(coll.size() == 1ul);
Acts::detail::pushBackOrInsertAtEnd(coll, 5ul);
BOOST_CHECK(coll.size() == 2ul);
std::size_t val = 1ul;
Acts::detail::pushBackOrInsertAtEnd(coll, val);
BOOST_CHECK(coll.size() == 3ul);

BOOST_CHECK(coll.find(2ul) != coll.end());
BOOST_CHECK(coll.find(5ul) != coll.end());
BOOST_CHECK(coll.find(1ul) != coll.end());
}

BOOST_AUTO_TEST_CASE(pushBackOrInsertAtEnd_unordered_set) {
std::unordered_set<std::size_t> coll;
Acts::detail::pushBackOrInsertAtEnd(coll, 2ul);
BOOST_CHECK(coll.size() == 1ul);
Acts::detail::pushBackOrInsertAtEnd(coll, 5ul);
BOOST_CHECK(coll.size() == 2ul);
std::size_t val = 1ul;
Acts::detail::pushBackOrInsertAtEnd(coll, val);
BOOST_CHECK(coll.size() == 3ul);

BOOST_CHECK(coll.find(2ul) != coll.end());
BOOST_CHECK(coll.find(5ul) != coll.end());
BOOST_CHECK(coll.find(1ul) != coll.end());
}

} // namespace Acts::Test
Loading

0 comments on commit 88c1223

Please sign in to comment.