From 6b950c634652be2e3dcdce3a08d9ff23a0caa54c Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 30 Oct 2024 21:13:16 +0100 Subject: [PATCH 1/4] chore: Update PR template, add cut-off marker (#3741) Kodiak handles this Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .github/pull_request_template.md | 44 ++++---------------------------- .kodiak.toml | 2 +- 2 files changed, 6 insertions(+), 40 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 9a17048cdd5..c2ece0d20f2 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,41 +1,7 @@ -PLEASE FOLLOW THE CHECKLIST BELOW WHEN CREATING A NEW PULL REQUEST. THE -CHECKLIST IS FOR YOUR INFORMATION AND MUST BE REMOVED BEFORE SUBMITTING THE PULL -REQUEST. +PLEASE DESCRIBE YOUR CHANGES. +THIS MESSAGE ENDS UP AS THE COMMIT MESSAGE. +DO NOT USE @-MENTIONS HERE! -## Checklist +--- END COMMIT MESSAGE --- -- [ ] Does the PR title follow the `: title` scheme? - - The prefix must be one of: - - - `fix`: for a bugfix - - `feat`: for a new feature - - `refactor`: for an improvement of an existing feature - - `perf`, `test`: for performance- or test-related changes - - `docs`: for documentation-related changes - - `build`, `ci`, `chore`: as appropriated for infrastructure changes - -- [ ] Does this modify the public API as defined in `docs/versioning.rst`? - - - [ ] Does the PR title contain a `!` to indicate a breaking change? - - [ ] Is there section starting with `BREAKING CHANGE:` in the PR body - that explains the breaking change? - -- [ ] Is the PR ready to be merged? - - - [ ] If not: is it marked as a draft PR? - -- [ ] Does this PR close an existing issue? - - - [ ] Is the issue correctly linked so it will be automatically closed - upon successful merge (See closing keywords link in the sidebar)? - -- The CI will initially report a missing milestone. One of the maintainers will - handle assigning a milestone for book-keeping. - -- An automated workflow will assign labels based on changed files, and whether - or not reference files were changed. These do not have to be set manually. - -- If you push updates, and you know they will be superseded later on, consider adding - `[skip ci]` in the commit message. This will instruct the CI system not to run any - jobs on this commit. +Any further description goes here, @-mentions are ok here! diff --git a/.kodiak.toml b/.kodiak.toml index b5be47df1ee..651905f7ce0 100644 --- a/.kodiak.toml +++ b/.kodiak.toml @@ -2,7 +2,7 @@ version = 1 merge.message.title = "pull_request_title" merge.message.body = "pull_request_body" -merge.message.cut_body_before = "--- BEGIN COMMIT MESSAGE ---" +merge.message.cut_body_after = "--- END COMMIT MESSAGE ---" merge.message.cut_body_and_text = true merge.method = "squash" merge.message.include_coauthors = true From 645e4bc386c9c7796d6003ef4584aeda3a7893d3 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 30 Oct 2024 21:21:20 +0100 Subject: [PATCH 2/4] fix: MeasSel error output, config check (#3794) I'm adding a explicit check on the input config here, because we default to an empty config, which is actually invalid, when it tries to run the selection itself. I'm also making the printing of the error code a bit more explicit in one case. From 0c57839b5b469b4ab585fe6a8254f06bb086ce7b Mon Sep 17 00:00:00 2001 From: Carlo Varni <75478407+CarloVarni@users.noreply.github.com> Date: Wed, 30 Oct 2024 22:39:14 +0100 Subject: [PATCH 3/4] refactor: Improve log in seeding (#3792) Adding some `VERBOSE` statements in the seeding, from Grid creation and Filling to seed finder and filter. --- Core/include/Acts/Seeding/SeedFilter.hpp | 14 ++++++++- Core/include/Acts/Seeding/SeedFilter.ipp | 20 +++++++++++-- Core/include/Acts/Seeding/SeedFinder.hpp | 15 ++++++++-- Core/include/Acts/Seeding/SeedFinder.ipp | 21 ++++++++++++-- .../detail/CylindricalSpacePointGrid.hpp | 10 +++++-- .../detail/CylindricalSpacePointGrid.ipp | 29 ++++++++++++++----- .../TrackFinding/src/SeedingAlgorithm.cpp | 15 ++++------ .../UnitTests/Core/Seeding/SeedFinderTest.cpp | 4 +-- 8 files changed, 100 insertions(+), 28 deletions(-) diff --git a/Core/include/Acts/Seeding/SeedFilter.hpp b/Core/include/Acts/Seeding/SeedFilter.hpp index 3c8720c42f0..94ceb17ae87 100644 --- a/Core/include/Acts/Seeding/SeedFilter.hpp +++ b/Core/include/Acts/Seeding/SeedFilter.hpp @@ -13,6 +13,7 @@ #include "Acts/Seeding/CandidatesForMiddleSp.hpp" #include "Acts/Seeding/IExperimentCuts.hpp" #include "Acts/Seeding/SeedFilterConfig.hpp" +#include "Acts/Utilities/Logger.hpp" #include #include @@ -37,8 +38,15 @@ struct SeedFilterState { template class SeedFilter final { public: - SeedFilter(SeedFilterConfig config, + SeedFilter(const SeedFilterConfig& config, IExperimentCuts* expCuts = nullptr); + SeedFilter(const SeedFilterConfig& config, + std::unique_ptr logger, + IExperimentCuts* expCuts = nullptr); + SeedFilter(const SeedFilter&) = delete; + SeedFilter& operator=(const SeedFilter&) = delete; + SeedFilter(SeedFilter&&) noexcept = default; + SeedFilter& operator=(SeedFilter&&) noexcept = default; SeedFilter() = delete; ~SeedFilter() = default; @@ -95,7 +103,11 @@ class SeedFilter final { } private: + const Logger& logger() const { return *m_logger; } + const SeedFilterConfig m_cfg; + std::unique_ptr m_logger = + Acts::getDefaultLogger("SeedFilter", Logging::Level::INFO); const IExperimentCuts* m_experimentCuts; }; } // namespace Acts diff --git a/Core/include/Acts/Seeding/SeedFilter.ipp b/Core/include/Acts/Seeding/SeedFilter.ipp index 5e54789b714..66498916a7e 100644 --- a/Core/include/Acts/Seeding/SeedFilter.ipp +++ b/Core/include/Acts/Seeding/SeedFilter.ipp @@ -16,7 +16,7 @@ namespace Acts { // constructor template SeedFilter::SeedFilter( - SeedFilterConfig config, + const SeedFilterConfig& config, IExperimentCuts* expCuts /* = 0*/) : m_cfg(config), m_experimentCuts(expCuts) { if (!config.isInInternalUnits) { @@ -24,6 +24,18 @@ SeedFilter::SeedFilter( "SeedFilterConfig not in ACTS internal units in SeedFilter"); } } + +template +SeedFilter::SeedFilter( + const SeedFilterConfig& config, std::unique_ptr logger, + IExperimentCuts* expCuts /* = 0*/) + : m_cfg(config), m_logger(std::move(logger)), m_experimentCuts(expCuts) { + if (!config.isInInternalUnits) { + throw std::runtime_error( + "SeedFilterConfig not in ACTS internal units in SeedFilter"); + } +} + // function to filter seeds based on all seeds with same bottom- and // middle-spacepoint. // return vector must contain weight of each seed @@ -295,10 +307,14 @@ void SeedFilter::filterSeeds_1SpFixed( seed.setVertexZ(zOrigin); seed.setQuality(bestSeedQuality); + ACTS_VERBOSE("Adding seed: [b=" << bottom->index() << ", m=" + << medium->index() << ", t=" << top->index() + << "], quality=" << bestSeedQuality + << ", vertexZ=" << zOrigin); Acts::detail::pushBackOrInsertAtEnd(outputCollection, std::move(seed)); - ++numTotalSeeds; } + ACTS_VERBOSE("Identified " << numTotalSeeds << " seeds"); } } // namespace Acts diff --git a/Core/include/Acts/Seeding/SeedFinder.hpp b/Core/include/Acts/Seeding/SeedFinder.hpp index cfe9908574c..cb5e7b06ff3 100644 --- a/Core/include/Acts/Seeding/SeedFinder.hpp +++ b/Core/include/Acts/Seeding/SeedFinder.hpp @@ -18,6 +18,7 @@ #include "Acts/Seeding/SeedFinderUtils.hpp" #include "Acts/Seeding/SpacePointGrid.hpp" #include "Acts/Seeding/detail/UtilityFunctions.hpp" +#include "Acts/Utilities/Logger.hpp" #include #include @@ -87,7 +88,14 @@ class SeedFinder { /// The only constructor. Requires a config object. /// @param config the configuration for the SeedFinder - SeedFinder(const Acts::SeedFinderConfig& config); + /// @param logger the ACTS logger + SeedFinder(const Acts::SeedFinderConfig& config, + std::unique_ptr logger = + getDefaultLogger("SeedFinder", Logging::Level::INFO)); + SeedFinder(SeedFinder&&) noexcept = + default; + SeedFinder& operator=(SeedFinder&&) noexcept = default; ~SeedFinder() = default; /** @name Disallow default instantiation, copy, assignment */ //@{ @@ -95,7 +103,7 @@ class SeedFinder { SeedFinder(const SeedFinder&) = delete; SeedFinder& operator=( - const SeedFinder&) = default; + const SeedFinder&) = delete; //@} /// Create all seeds from the space points in the three iterators. @@ -172,7 +180,10 @@ class SeedFinder { SeedingState& state) const; private: + const Logger& logger() const { return *m_logger; } + Acts::SeedFinderConfig m_config; + std::unique_ptr m_logger{nullptr}; }; } // namespace Acts diff --git a/Core/include/Acts/Seeding/SeedFinder.ipp b/Core/include/Acts/Seeding/SeedFinder.ipp index 45b95ec05d5..e6ccb9f589f 100644 --- a/Core/include/Acts/Seeding/SeedFinder.ipp +++ b/Core/include/Acts/Seeding/SeedFinder.ipp @@ -15,8 +15,9 @@ namespace Acts { template SeedFinder::SeedFinder( - const Acts::SeedFinderConfig& config) - : m_config(config) { + const Acts::SeedFinderConfig& config, + std::unique_ptr logger) + : m_config(config), m_logger(std::move(logger)) { if (!config.isInInternalUnits) { throw std::runtime_error( "SeedFinderConfig not in ACTS internal units in SeedFinder"); @@ -110,6 +111,12 @@ void SeedFinder::createSeedsForGroup( // same z-bin auto [minRadiusRangeForMiddle, maxRadiusRangeForMiddle] = retrieveRadiusRangeForMiddle(*middleSPs.front(), rMiddleSPRange); + ACTS_VERBOSE("Current global bin: " << middleSPsIdx << ", z value of " + << middleSPs.front()->z()); + ACTS_VERBOSE("Validity range (radius) for the middle space point is [" + << minRadiusRangeForMiddle << ", " << maxRadiusRangeForMiddle + << "]"); + for (const external_spacepoint_t* spM : middleSPs) { const float rM = spM->radius(); @@ -136,6 +143,7 @@ void SeedFinder::createSeedsForGroup( // no top SP found -> try next spM if (state.compatTopSP.empty()) { + ACTS_VERBOSE("No compatible Tops, moving to next middle candidate"); continue; } @@ -157,6 +165,10 @@ void SeedFinder::createSeedsForGroup( seedFilterState.rMaxSeedConf = seedConfRange.rMaxSeedConf; // continue if number of top SPs is smaller than minimum if (state.compatTopSP.size() < seedFilterState.nTopSeedConf) { + ACTS_VERBOSE( + "Number of top SPs is " + << state.compatTopSP.size() + << " and is smaller than minimum, moving to next middle candidate"); continue; } } @@ -170,9 +182,14 @@ void SeedFinder::createSeedsForGroup( // no bottom SP found -> try next spM if (state.compatBottomSP.empty()) { + ACTS_VERBOSE("No compatible Bottoms, moving to next middle candidate"); continue; } + ACTS_VERBOSE("Candidates: " << state.compatBottomSP.size() + << " bottoms and " << state.compatTopSP.size() + << " tops for middle candidate indexed " + << spM->index()); // filter candidates if (m_config.useDetailedDoubleMeasurementInfo) { filterCandidates( diff --git a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp index 4410f1c125b..a2a214fa39b 100644 --- a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp +++ b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.hpp @@ -11,6 +11,7 @@ #include "Acts/Seeding/BinnedGroup.hpp" #include "Acts/Seeding/SeedFinderConfig.hpp" #include "Acts/Utilities/Grid.hpp" +#include "Acts/Utilities/Logger.hpp" #include #include @@ -158,7 +159,8 @@ class CylindricalSpacePointGridCreator { template static Acts::CylindricalSpacePointGrid createGrid( const Acts::CylindricalSpacePointGridConfig& _config, - const Acts::CylindricalSpacePointGridOptions& _options); + const Acts::CylindricalSpacePointGridOptions& _options, + const Acts::Logger& logger = Acts::getDummyLogger()); template @@ -167,7 +169,8 @@ class CylindricalSpacePointGridCreator { const Acts::SeedFinderOptions& options, Acts::CylindricalSpacePointGrid& grid, external_spacepoint_iterator_t spBegin, - external_spacepoint_iterator_t spEnd); + external_spacepoint_iterator_t spEnd, + const Acts::Logger& logger = Acts::getDummyLogger()); template requires std::ranges::range && @@ -177,7 +180,8 @@ class CylindricalSpacePointGridCreator { const Acts::SeedFinderConfig& config, const Acts::SeedFinderOptions& options, Acts::CylindricalSpacePointGrid& grid, - const external_collection_t& collection); + const external_collection_t& collection, + const Acts::Logger& logger = Acts::getDummyLogger()); }; } // namespace Acts diff --git a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp index 523d470eae5..b99cdbdfaf9 100644 --- a/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp +++ b/Core/include/Acts/Seeding/detail/CylindricalSpacePointGrid.ipp @@ -12,7 +12,8 @@ template Acts::CylindricalSpacePointGrid Acts::CylindricalSpacePointGridCreator::createGrid( const Acts::CylindricalSpacePointGridConfig& config, - const Acts::CylindricalSpacePointGridOptions& options) { + const Acts::CylindricalSpacePointGridOptions& options, + const Acts::Logger& logger) { if (!config.isInInternalUnits) { throw std::runtime_error( "CylindricalSpacePointGridConfig not in ACTS internal units in " @@ -30,6 +31,9 @@ Acts::CylindricalSpacePointGridCreator::createGrid( // for no magnetic field, create 100 phi-bins if (options.bFieldInZ == 0) { phiBins = 100; + ACTS_VERBOSE( + "B-Field is 0 (z-coordinate), setting the number of bins in phi to " + << phiBins); } else { // calculate circle intersections of helix and max detector radius float minHelixRadius = @@ -39,7 +43,7 @@ Acts::CylindricalSpacePointGridCreator::createGrid( // = pT[MeV] / (300 *Bz[kT]) // sanity check: if yOuter takes the square root of a negative number - if (minHelixRadius < config.rMax / 2) { + if (minHelixRadius < config.rMax * 0.5) { throw std::domain_error( "The value of minHelixRadius cannot be smaller than rMax / 2. Please " "check the configuration of bFieldInZ and minPt"); @@ -141,6 +145,12 @@ Acts::CylindricalSpacePointGridCreator::createGrid( Axis zAxis(std::move(zValues)); Axis rAxis(std::move(rValues)); + + ACTS_VERBOSE("Defining Grid:"); + ACTS_VERBOSE("- Phi Axis: " << phiAxis); + ACTS_VERBOSE("- Z axis : " << zAxis); + ACTS_VERBOSE("- R axis : " << rAxis); + return Acts::CylindricalSpacePointGrid( std::make_tuple(std::move(phiAxis), std::move(zAxis), std::move(rAxis))); } @@ -152,7 +162,7 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( const Acts::SeedFinderOptions& options, Acts::CylindricalSpacePointGrid& grid, external_spacepoint_iterator_t spBegin, - external_spacepoint_iterator_t spEnd) { + external_spacepoint_iterator_t spEnd, const Acts::Logger& logger) { if (!config.isInInternalUnits) { throw std::runtime_error( "SeedFinderConfig not in ACTS internal units in BinnedSPGroup"); @@ -180,9 +190,10 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( std::vector rBinsIndex; rBinsIndex.reserve(grid.size()); + ACTS_VERBOSE("Fetching " << std::distance(spBegin, spEnd) + << " space points to the grid"); std::size_t counter = 0ul; - for (external_spacepoint_iterator_t it = spBegin; it != spEnd; - it++, ++counter) { + for (external_spacepoint_iterator_t it = spBegin; it != spEnd; ++it) { const external_spacepoint_t& sp = *it; // remove SPs according to experiment specific cuts @@ -199,6 +210,7 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( std::size_t globIndex = grid.globalBinFromPosition(position); auto& rbin = grid.at(globIndex); rbin.push_back(&sp); + ++counter; // keep track of the bins we modify so that we can later sort the SPs in // those bins only @@ -213,6 +225,9 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( auto& rbin = grid.atPosition(binIndex); std::ranges::sort(rbin, {}, [](const auto& rb) { return rb->radius(); }); } + + ACTS_VERBOSE( + "Number of space points inserted (within grid range): " << counter); } template @@ -223,8 +238,8 @@ void Acts::CylindricalSpacePointGridCreator::fillGrid( const Acts::SeedFinderConfig& config, const Acts::SeedFinderOptions& options, Acts::CylindricalSpacePointGrid& grid, - const external_collection_t& collection) { + const external_collection_t& collection, const Acts::Logger& logger) { Acts::CylindricalSpacePointGridCreator::fillGrid( config, options, grid, std::ranges::begin(collection), - std::ranges::end(collection)); + std::ranges::end(collection), logger); } diff --git a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp index ed5b11e4df2..2cea4b7b505 100644 --- a/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp +++ b/Examples/Algorithms/TrackFinding/src/SeedingAlgorithm.cpp @@ -45,9 +45,8 @@ ActsExamples::SeedingAlgorithm::SeedingAlgorithm( // internal units m_cfg.seedFilterConfig = m_cfg.seedFilterConfig.toInternalUnits(); m_cfg.seedFinderConfig.seedFilter = - std::make_shared>( - m_cfg.seedFilterConfig); - + std::make_unique>( + m_cfg.seedFilterConfig, logger().cloneWithSuffix("SeedFilter")); m_cfg.seedFinderConfig = m_cfg.seedFinderConfig.toInternalUnits().calculateDerivedQuantities(); m_cfg.seedFinderOptions = @@ -195,13 +194,10 @@ ActsExamples::SeedingAlgorithm::SeedingAlgorithm( m_topBinFinder = std::make_unique>( m_cfg.numPhiNeighbors, m_cfg.zBinNeighborsTop, 0); - m_cfg.seedFinderConfig.seedFilter = - std::make_unique>( - m_cfg.seedFilterConfig); m_seedFinder = Acts::SeedFinder>( - m_cfg.seedFinderConfig); + m_cfg.seedFinderConfig, logger().cloneWithSuffix("SeedFinder")); } ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute( @@ -244,10 +240,11 @@ ActsExamples::ProcessCode ActsExamples::SeedingAlgorithm::execute( Acts::CylindricalSpacePointGrid grid = Acts::CylindricalSpacePointGridCreator::createGrid( - m_cfg.gridConfig, m_cfg.gridOptions); + m_cfg.gridConfig, m_cfg.gridOptions, logger()); Acts::CylindricalSpacePointGridCreator::fillGrid( - m_cfg.seedFinderConfig, m_cfg.seedFinderOptions, grid, spContainer); + m_cfg.seedFinderConfig, m_cfg.seedFinderOptions, grid, spContainer, + logger()); // Compute radius Range // we rely on the fact the grid is storing the proxies diff --git a/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp b/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp index 9d89c1c96c6..8368bdbc3f2 100644 --- a/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp +++ b/Tests/UnitTests/Core/Seeding/SeedFinderTest.cpp @@ -186,8 +186,8 @@ int main(int argc, char** argv) { Acts::SeedFilterConfig sfconf; Acts::ATLASCuts atlasCuts = Acts::ATLASCuts(); - config.seedFilter = std::make_unique>( - Acts::SeedFilter(sfconf, &atlasCuts)); + config.seedFilter = + std::make_unique>(sfconf, &atlasCuts); Acts::SeedFinder> a; // test creation of unconfigured finder a = Acts::SeedFinder>( From e2789b66fa67fe419b86d74ef2d3d86562c8949b Mon Sep 17 00:00:00 2001 From: Benjamin Huth <37871400+benjaminhuth@users.noreply.github.com> Date: Wed, 30 Oct 2024 23:57:22 +0100 Subject: [PATCH 4/4] fix: Require TBB to be found by cmake (#3507) Previously, the configuration could trun through even if TBB is not present on the system. --- CMakeLists.txt | 1 - Examples/Framework/CMakeLists.txt | 31 +++----- .../ActsExamples/Utilities/tbbWrap.hpp | 70 ++----------------- .../Framework/src/Framework/Sequencer.cpp | 10 +-- docs/getting_started.md | 1 - 5 files changed, 13 insertions(+), 100 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f81002cab83..f5209e0d210 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,7 +73,6 @@ option(ACTS_BUILD_EXAMPLES_HEPMC3 "Build HepMC3-based code in the examples" OFF) option(ACTS_BUILD_EXAMPLES_HASHING "Build Hashing-based code in the examples" OFF) option(ACTS_BUILD_EXAMPLES_PYTHIA8 "Build Pythia8-based code in the examples" OFF) option(ACTS_BUILD_EXAMPLES_PYTHON_BINDINGS "Build python bindings for the examples" OFF) -option(ACTS_USE_EXAMPLES_TBB "Use Threading Building Blocks library in the examples" ON) option(ACTS_BUILD_ANALYSIS_APPS "Build Analysis applications in the examples" OFF) # test related options option(ACTS_BUILD_BENCHMARKS "Build benchmarks" OFF) diff --git a/Examples/Framework/CMakeLists.txt b/Examples/Framework/CMakeLists.txt index efe06302800..db45cffc925 100644 --- a/Examples/Framework/CMakeLists.txt +++ b/Examples/Framework/CMakeLists.txt @@ -46,30 +46,15 @@ target_compile_definitions( PRIVATE BOOST_FILESYSTEM_NO_DEPRECATED ) -if(ACTS_USE_EXAMPLES_TBB) - # newer DD4hep version require TBB and search internally for TBB in - # config-only mode. to avoid mismatches we explicitly search using - # config-only mode first to be sure that we find the same version. - find_package(TBB ${_acts_tbb_version} CONFIG) - if(NOT TBB_FOUND) - # no version check possible when using the find module - find_package(TBB ${_acts_tbb_version} MODULE) - endif() -else() - set(TBB_FOUND FALSE) -endif() -if(TBB_FOUND) - target_link_libraries(ActsExamplesFramework PUBLIC TBB::tbb) -else() - message( - STATUS - "disable TBB for Examples/Framework - only single-threaded running will be supported" - ) - target_compile_definitions( - ActsExamplesFramework - PUBLIC -DACTS_EXAMPLES_NO_TBB - ) +# newer DD4hep version require TBB and search internally for TBB in +# config-only mode. to avoid mismatches we explicitly search using +# config-only mode first to be sure that we find the same version. +find_package(TBB ${_acts_tbb_version} CONFIG) +if(NOT TBB_FOUND) + # no version check possible when using the find module + find_package(TBB ${_acts_tbb_version} MODULE REQUIRED) endif() +target_link_libraries(ActsExamplesFramework PUBLIC TBB::tbb) install( TARGETS ActsExamplesFramework diff --git a/Examples/Framework/include/ActsExamples/Utilities/tbbWrap.hpp b/Examples/Framework/include/ActsExamples/Utilities/tbbWrap.hpp index 84784c55d31..79969369221 100644 --- a/Examples/Framework/include/ActsExamples/Utilities/tbbWrap.hpp +++ b/Examples/Framework/include/ActsExamples/Utilities/tbbWrap.hpp @@ -8,20 +8,11 @@ #pragma once -// uncomment to remove all use of tbb library. -// #define ACTS_EXAMPLES_NO_TBB - -#ifdef ACTS_EXAMPLES_NO_TBB -#define ACTS_EXAMPLES_WITH_TBB(a) -#include -#else -#define ACTS_EXAMPLES_WITH_TBB(a) a #include #include #include #include -#endif /// Wrapper for most of the tbb functions that we use in Sequencer. /// @@ -30,34 +21,9 @@ /// tbb::blocked_range (which doesn't require any thread setup) is still taken /// from the tbb library. /// -/// However, if ACTS_EXAMPLES_NO_TBB is defined, then don't use tbb library at -/// all (requires nthreads=1 or -1). This allows the ACTS Examples to be built -/// without the tbb library (and reduces the dependency on ROOT). -/// In this case, we provide our own minimal implementation of -/// tbb::blocked_range. -/// /// Based on an idea from /// https://stackoverflow.com/questions/59736661/how-to-completely-switch-off-threading-in-tbb-code -#ifdef ACTS_EXAMPLES_NO_TBB -namespace ActsExamples::tbb { -namespace task_arena { -constexpr int automatic = -1; -} // namespace task_arena - -template -struct blocked_range { - blocked_range(Value begin_, Value end_) : my_end(end_), my_begin(begin_) {} - Value begin() const { return my_begin; } - Value end() const { return my_end; } - - private: - Value my_end; - Value my_begin; -}; -} // namespace ActsExamples::tbb -#endif - namespace ActsExamples::tbbWrap { /// enableTBB keeps a record of whether we are multi-threaded (nthreads!=1) or /// not. This is set once in task_arena and stored globally. @@ -67,17 +33,10 @@ namespace ActsExamples::tbbWrap { static bool enableTBB(int nthreads = -99) { static bool setting = false; if (nthreads != -99) { -#ifdef ACTS_EXAMPLES_NO_TBB - if (nthreads > 1) { - throw std::runtime_error( - "tbb is not available, so can't do multi-threading."); - } -#else bool newSetting = (nthreads != 1); if (!setting && newSetting) { setting = newSetting; } -#endif } return setting; } @@ -87,28 +46,20 @@ static bool enableTBB(int nthreads = -99) { /// That should be fine because the task_arena is initialised before spawning /// any threads. class task_arena { -#ifndef ACTS_EXAMPLES_NO_TBB std::optional tbb; -#endif public: - task_arena(int nthreads = tbb::task_arena::automatic, - unsigned ACTS_EXAMPLES_WITH_TBB(res) = 1) { + task_arena(int nthreads = tbb::task_arena::automatic, unsigned res = 1) { if (enableTBB(nthreads)) { -#ifndef ACTS_EXAMPLES_NO_TBB tbb.emplace(nthreads, res); -#endif } } template void execute(const F& f) { -#ifndef ACTS_EXAMPLES_NO_TBB if (tbb) { tbb->execute(f); - } else -#endif - { + } else { f(); } } @@ -119,12 +70,9 @@ class parallel_for { public: template parallel_for(const R& r, const F& f) { -#ifndef ACTS_EXAMPLES_NO_TBB if (enableTBB()) { tbb::parallel_for(r, f); - } else -#endif - { + } else { for (auto i = r.begin(); i != r.end(); ++i) { // use default grainsize=1 f(R(i, i + 1)); } @@ -134,39 +82,29 @@ class parallel_for { /// Small wrapper for tbb::queuing_mutex and tbb::queuing_mutex::scoped_lock. class queuing_mutex { -#ifndef ACTS_EXAMPLES_NO_TBB std::optional tbb; -#endif public: queuing_mutex() { -#ifndef ACTS_EXAMPLES_NO_TBB if (enableTBB()) { tbb.emplace(); } -#endif } class scoped_lock { -#ifndef ACTS_EXAMPLES_NO_TBB std::optional tbb; -#endif public: scoped_lock() { -#ifndef ACTS_EXAMPLES_NO_TBB if (enableTBB()) { tbb.emplace(); } -#endif } - explicit scoped_lock(queuing_mutex& ACTS_EXAMPLES_WITH_TBB(m)) { -#ifndef ACTS_EXAMPLES_NO_TBB + explicit scoped_lock(queuing_mutex& m) { if (enableTBB()) { tbb.emplace(*m.tbb); } -#endif } }; }; diff --git a/Examples/Framework/src/Framework/Sequencer.cpp b/Examples/Framework/src/Framework/Sequencer.cpp index 5c1a76b5bd4..1c6bbaa97fb 100644 --- a/Examples/Framework/src/Framework/Sequencer.cpp +++ b/Examples/Framework/src/Framework/Sequencer.cpp @@ -42,15 +42,11 @@ #include #include -#include - -#ifndef ACTS_EXAMPLES_NO_TBB #include -#endif - #include #include #include +#include namespace ActsExamples { @@ -108,16 +104,12 @@ Sequencer::Sequencer(const Sequencer::Config& cfg) m_taskArena((m_cfg.numThreads < 0) ? tbb::task_arena::automatic : m_cfg.numThreads), m_logger(Acts::getDefaultLogger("Sequencer", m_cfg.logLevel)) { -#ifndef ACTS_EXAMPLES_NO_TBB if (m_cfg.numThreads == 1) { -#endif ACTS_INFO("Create Sequencer (single-threaded)"); -#ifndef ACTS_EXAMPLES_NO_TBB } else { ROOT::EnableThreadSafety(); ACTS_INFO("Create Sequencer with " << m_cfg.numThreads << " threads"); } -#endif const char* envvar = std::getenv("ACTS_SEQUENCER_DISABLE_FPEMON"); if (envvar != nullptr) { diff --git a/docs/getting_started.md b/docs/getting_started.md index f17ec5abcf1..aff8e38a3b0 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -301,7 +301,6 @@ components. | ACTS_BUILD_EXAMPLES_HASHING | Build Hashing-based code in the examples
type: `bool`, default: `OFF` | | ACTS_BUILD_EXAMPLES_PYTHIA8 | Build Pythia8-based code in the examples
type: `bool`, default: `OFF` | | ACTS_BUILD_EXAMPLES_PYTHON_BINDINGS | Build python bindings for the examples
type: `bool`, default: `OFF` | -| ACTS_USE_EXAMPLES_TBB | Use Threading Building Blocks library in
the examples
type: `bool`, default: `ON` | | ACTS_BUILD_ANALYSIS_APPS | Build Analysis applications in the
examples
type: `bool`, default: `OFF` | | ACTS_BUILD_BENCHMARKS | Build benchmarks
type: `bool`, default: `OFF` | | ACTS_BUILD_INTEGRATIONTESTS | Build integration tests
type: `bool`, default: `OFF` |