From f88815a57713028f42c1b3f2495aa4faf2debc97 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Thu, 9 Jan 2025 18:42:15 +0100 Subject: [PATCH 1/9] add buffered reader wrapper --- Examples/Framework/CMakeLists.txt | 1 + .../ActsExamples/Framework/BufferedReader.hpp | 73 ++++++++++++++++++ .../Framework/SequenceElement.hpp | 2 + .../ActsExamples/Framework/WhiteBoard.hpp | 11 ++- .../src/Framework/BufferedReader.cpp | 75 +++++++++++++++++++ .../Framework/src/Framework/WhiteBoard.cpp | 11 +++ Examples/Python/src/Input.cpp | 6 ++ 7 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp create mode 100644 Examples/Framework/src/Framework/BufferedReader.cpp diff --git a/Examples/Framework/CMakeLists.txt b/Examples/Framework/CMakeLists.txt index 032627e0b89..26e280b4dd3 100644 --- a/Examples/Framework/CMakeLists.txt +++ b/Examples/Framework/CMakeLists.txt @@ -15,6 +15,7 @@ add_library( src/Framework/RandomNumbers.cpp src/Framework/Sequencer.cpp src/Framework/DataHandle.cpp + src/Framework/BufferedReader.cpp src/Utilities/EventDataTransforms.cpp src/Utilities/Paths.cpp src/Utilities/Options.cpp diff --git a/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp b/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp new file mode 100644 index 00000000000..1cf80c6fa5a --- /dev/null +++ b/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp @@ -0,0 +1,73 @@ +// This file is part of the ACTS project. +// +// Copyright (C) 2016 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 https://mozilla.org/MPL/2.0/. + +#pragma once + +#include "Acts/Utilities/Logger.hpp" +#include "ActsExamples/Framework/AlgorithmContext.hpp" +#include "ActsExamples/Framework/IReader.hpp" +#include "ActsExamples/Framework/ProcessCode.hpp" + +#include + +namespace ActsExamples { + +class WhiteBoard; + +/// Event data reader that takes a concrete reader instance, reads a number of +/// events in a buffer, and selects events from that buffer instead of directly +/// reading them from disk. +/// The purpose is to avoid IO bottlenecks in timing measurements +class BufferedReader final : public IReader { + public: + struct Config { + /// The downstream reader that should be used + std::shared_ptr downstreamReader; + + /// The seed for sampling events from the buffer + std::size_t selectionSeed = 123456; + + /// Buffer size. The reader will throw and exception if the downstream + /// reader does not provide enough events + std::size_t bufferSize; + }; + + /// Constructed the reader + BufferedReader(const Config& config, Acts::Logging::Level level); + + /// Return the config + const Config& config() const { return m_cfg; } + + /// Give the reader a understandable name + std::string name() const override { + return "Buffered" + m_cfg.downstreamReader->name(); + } + + /// The buffered reader provides the maximum available event range + std::pair availableEvents() const override { + return {0, std::numeric_limits::max()}; + } + + /// Return a event from the buffer + ProcessCode read(const AlgorithmContext& context) override; + + /// Fulfill the algorithm interface + ProcessCode initialize() override { return ProcessCode::SUCCESS; } + + /// Fulfill the algorithm interface + ProcessCode finalize() override { return ProcessCode::SUCCESS; } + + private: + Config m_cfg; + std::unique_ptr m_logger; + std::vector> m_buffer; + + const Acts::Logger& logger() const { return *m_logger; } +}; + +} // namespace ActsExamples diff --git a/Examples/Framework/include/ActsExamples/Framework/SequenceElement.hpp b/Examples/Framework/include/ActsExamples/Framework/SequenceElement.hpp index 4db3b8d1af6..591579af2cd 100644 --- a/Examples/Framework/include/ActsExamples/Framework/SequenceElement.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/SequenceElement.hpp @@ -51,6 +51,8 @@ class SequenceElement { template friend class ReadDataHandle; + friend class BufferedReader; + std::vector m_writeHandles; std::vector m_readHandles; }; diff --git a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp index 777d27fe898..660059841a9 100644 --- a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp @@ -38,12 +38,21 @@ class WhiteBoard { Acts::getDefaultLogger("WhiteBoard", Acts::Logging::INFO), std::unordered_map objectAliases = {}); - // A WhiteBoard holds unique elements and can not be copied WhiteBoard(const WhiteBoard& other) = delete; WhiteBoard& operator=(const WhiteBoard&) = delete; + WhiteBoard(WhiteBoard&& other) = default; + WhiteBoard& operator=(WhiteBoard&& other) = default; + bool exists(const std::string& name) const; + /// Adds the data of this whiteboard instance to another whiteboard. + /// This is a low overhead operation, since the data holders are + /// shared pointers. + /// Throws an exception if the other whiteboard already contains one of + /// the keys in this whiteboard. + void shareDataWith(WhiteBoard& other) const; + private: /// Store an object on the white board and transfer ownership. /// diff --git a/Examples/Framework/src/Framework/BufferedReader.cpp b/Examples/Framework/src/Framework/BufferedReader.cpp new file mode 100644 index 00000000000..5d8902dca69 --- /dev/null +++ b/Examples/Framework/src/Framework/BufferedReader.cpp @@ -0,0 +1,75 @@ +// This file is part of the ACTS project. +// +// Copyright (C) 2016 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 https://mozilla.org/MPL/2.0/. + +#include "ActsExamples/Framework/BufferedReader.hpp" + +#include "Acts/Utilities/Logger.hpp" +#include "ActsExamples/Framework/AlgorithmContext.hpp" +#include "ActsExamples/Framework/WhiteBoard.hpp" + +#include +#include + +namespace ActsExamples { + +BufferedReader::BufferedReader(const Config &config, Acts::Logging::Level level) + : m_cfg(config), m_logger(Acts::getDefaultLogger(name(), level)) { + if (!m_cfg.downstreamReader) { + throw std::invalid_argument("No downstream reader provided!"); + } + + // Register write and read handles of the downstream reader + for (auto rh : m_cfg.downstreamReader->readHandles()) { + registerReadHandle(*rh); + } + + for (auto wh : m_cfg.downstreamReader->writeHandles()) { + registerWriteHandle(*wh); + } + + // Read the events + auto [ebegin, eend] = m_cfg.downstreamReader->availableEvents(); + if (eend - ebegin < m_cfg.bufferSize) { + throw std::runtime_error("Reader does not provide enough events"); + } + + ACTS_INFO("Start reading events into buffer..."); + + m_buffer.reserve(eend - ebegin); + for (auto i = ebegin; i < ebegin + m_cfg.bufferSize; ++i) { + auto board = std::make_unique(m_logger->clone()); + ActsExamples::AlgorithmContext ctx(0, i, *board); + + ACTS_DEBUG("Read event " << i << " into buffer"); + m_cfg.downstreamReader->read(ctx); + m_buffer.emplace_back(std::move(board)); + } + + ACTS_INFO("Filled " << m_buffer.size() << " events into the buffer"); +} + +ProcessCode BufferedReader::read(const AlgorithmContext &ctx) { + // Set up a random event selection that is consistent if multiple + // BufferedReader are used within a workflow The linear congruential engine is + // chosen since it is cheap to instantiate. For each eventNumber, it is put in + // a reproducible state. + std::minstd_rand rng(m_cfg.selectionSeed); + rng.discard(ctx.eventNumber); + + /// Sample from the buffer and transfer the content + std::uniform_int_distribution dist(0, m_cfg.bufferSize - 1); + + const auto entry = dist(rng); + m_buffer.at(entry)->shareDataWith(ctx.eventStore); + + ACTS_DEBUG("Use buffer entry " << entry << " for event " << ctx.eventNumber); + + return ProcessCode::SUCCESS; +} + +} // namespace ActsExamples diff --git a/Examples/Framework/src/Framework/WhiteBoard.cpp b/Examples/Framework/src/Framework/WhiteBoard.cpp index 099eac53efc..c8ccb2a896e 100644 --- a/Examples/Framework/src/Framework/WhiteBoard.cpp +++ b/Examples/Framework/src/Framework/WhiteBoard.cpp @@ -84,3 +84,14 @@ std::string ActsExamples::WhiteBoard::typeMismatchMessage( boost::core::demangle(req) + " but actually " + boost::core::demangle(act)}; } + +void ActsExamples::WhiteBoard::shareDataWith(WhiteBoard &other) const { + for (auto &[key, val] : m_store) { + auto [it, success] = other.m_store.insert({key, val}); + + if (!success) { + throw std::runtime_error("Cannot share key '" + key + + "', is already present"); + } + } +} diff --git a/Examples/Python/src/Input.cpp b/Examples/Python/src/Input.cpp index b3ad9e1f657..70920152c6f 100644 --- a/Examples/Python/src/Input.cpp +++ b/Examples/Python/src/Input.cpp @@ -8,6 +8,7 @@ #include "Acts/Plugins/Python/Utilities.hpp" #include "ActsExamples/EventData/Cluster.hpp" +#include "ActsExamples/Framework/BufferedReader.hpp" #include "ActsExamples/Io/Csv/CsvDriftCircleReader.hpp" #include "ActsExamples/Io/Csv/CsvExaTrkXGraphReader.hpp" #include "ActsExamples/Io/Csv/CsvMeasurementReader.hpp" @@ -39,6 +40,11 @@ namespace Acts::Python { void addInput(Context& ctx) { auto mex = ctx.get("examples"); + // Buffered reader + ACTS_PYTHON_DECLARE_READER(ActsExamples::BufferedReader, mex, + "BufferedReader", downstreamReader, selectionSeed, + bufferSize); + // ROOT READERS ACTS_PYTHON_DECLARE_READER(ActsExamples::RootParticleReader, mex, "RootParticleReader", outputParticles, treeName, From ffcc912c342b9e704783f8a5ed76053e2d983ed7 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Fri, 10 Jan 2025 11:19:27 +0100 Subject: [PATCH 2/9] add test --- Examples/Python/tests/test_reader.py | 46 ++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/Examples/Python/tests/test_reader.py b/Examples/Python/tests/test_reader.py index 46b1c9c229f..20099f4409d 100644 --- a/Examples/Python/tests/test_reader.py +++ b/Examples/Python/tests/test_reader.py @@ -435,3 +435,49 @@ def test_edm4hep_tracks_reader(tmp_path): ) s.run() + + +@pytest.mark.root +def test_buffered_reader(tmp_path, conf_const, ptcl_gun): + # Test the buffered reader with the ROOT particle reader + # need to write out some particles first + s = Sequencer(numThreads=1, events=10, logLevel=acts.logging.WARNING) + evGen = ptcl_gun(s) + + file = tmp_path / "particles.root" + s.addWriter( + conf_const( + RootParticleWriter, + acts.logging.WARNING, + inputParticles=evGen.config.outputParticles, + filePath=str(file), + ) + ) + + s.run() + + # reset sequencer for reading + s2 = Sequencer(numThreads=1, logLevel=acts.logging.WARNING) + + reader = acts.examples.RootParticleReader( + level=acts.logging.WARNING, + outputParticles="particles_input", + filePath=str(file), + ) + + s2.addReader( + acts.examples.BufferedReader( + level=acts.logging.WARNING, + downstreamReader=reader, + bufferSize=10, + ) + ) + + alg = AssertCollectionExistsAlg( + "particles_input", "check_alg", acts.logging.WARNING + ) + s2.addAlgorithm(alg) + + s2.run() + + assert alg.events_seen == 10 From a83827e49d0df9d88cb0ae5b052c91363cb35971 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Fri, 10 Jan 2025 13:03:24 +0100 Subject: [PATCH 3/9] update --- Examples/Python/tests/test_reader.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Examples/Python/tests/test_reader.py b/Examples/Python/tests/test_reader.py index 20099f4409d..edaed74f001 100644 --- a/Examples/Python/tests/test_reader.py +++ b/Examples/Python/tests/test_reader.py @@ -441,7 +441,10 @@ def test_edm4hep_tracks_reader(tmp_path): def test_buffered_reader(tmp_path, conf_const, ptcl_gun): # Test the buffered reader with the ROOT particle reader # need to write out some particles first - s = Sequencer(numThreads=1, events=10, logLevel=acts.logging.WARNING) + eventsInBuffer = 5 + eventsToProcess = 10 + + s = Sequencer(numThreads=1, events=eventsInBuffer, logLevel=acts.logging.WARNING) evGen = ptcl_gun(s) file = tmp_path / "particles.root" @@ -457,7 +460,7 @@ def test_buffered_reader(tmp_path, conf_const, ptcl_gun): s.run() # reset sequencer for reading - s2 = Sequencer(numThreads=1, logLevel=acts.logging.WARNING) + s2 = Sequencer(events=eventsToProcess, numThreads=1, logLevel=acts.logging.WARNING) reader = acts.examples.RootParticleReader( level=acts.logging.WARNING, @@ -469,7 +472,7 @@ def test_buffered_reader(tmp_path, conf_const, ptcl_gun): acts.examples.BufferedReader( level=acts.logging.WARNING, downstreamReader=reader, - bufferSize=10, + bufferSize=eventsInBuffer, ) ) @@ -480,4 +483,4 @@ def test_buffered_reader(tmp_path, conf_const, ptcl_gun): s2.run() - assert alg.events_seen == 10 + assert alg.events_seen == eventsToProcess From a265a08483c3144f1802f897e8a987a7d57e40a6 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Thu, 16 Jan 2025 12:52:25 +0100 Subject: [PATCH 4/9] feedback --- .../ActsExamples/Framework/BufferedReader.hpp | 8 ++++---- .../include/ActsExamples/Framework/WhiteBoard.hpp | 9 ++++----- .../Framework/src/Framework/BufferedReader.cpp | 14 +++++++------- Examples/Framework/src/Framework/WhiteBoard.cpp | 9 +++++---- Examples/Python/src/Input.cpp | 2 +- Examples/Python/tests/test_reader.py | 2 +- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp b/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp index 1cf80c6fa5a..75a884163d4 100644 --- a/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp @@ -26,15 +26,15 @@ class WhiteBoard; class BufferedReader final : public IReader { public: struct Config { - /// The downstream reader that should be used - std::shared_ptr downstreamReader; + /// The upstream reader that should be used + std::shared_ptr upstreamReader; /// The seed for sampling events from the buffer std::size_t selectionSeed = 123456; /// Buffer size. The reader will throw and exception if the downstream /// reader does not provide enough events - std::size_t bufferSize; + std::size_t bufferSize = 1; }; /// Constructed the reader @@ -45,7 +45,7 @@ class BufferedReader final : public IReader { /// Give the reader a understandable name std::string name() const override { - return "Buffered" + m_cfg.downstreamReader->name(); + return "Buffered" + m_cfg.upstreamReader->name(); } /// The buffered reader provides the maximum available event range diff --git a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp index 660059841a9..84170b44b91 100644 --- a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -46,12 +45,12 @@ class WhiteBoard { bool exists(const std::string& name) const; - /// Adds the data of this whiteboard instance to another whiteboard. + /// Copies key from another whiteboard to this whiteboard. /// This is a low overhead operation, since the data holders are /// shared pointers. - /// Throws an exception if the other whiteboard already contains one of - /// the keys in this whiteboard. - void shareDataWith(WhiteBoard& other) const; + /// Throws an exception if this whiteboard already contains one of + /// the keys in the other whiteboard. + void copyFrom(const WhiteBoard& other); private: /// Store an object on the white board and transfer ownership. diff --git a/Examples/Framework/src/Framework/BufferedReader.cpp b/Examples/Framework/src/Framework/BufferedReader.cpp index 5d8902dca69..597ff83b7e5 100644 --- a/Examples/Framework/src/Framework/BufferedReader.cpp +++ b/Examples/Framework/src/Framework/BufferedReader.cpp @@ -19,21 +19,21 @@ namespace ActsExamples { BufferedReader::BufferedReader(const Config &config, Acts::Logging::Level level) : m_cfg(config), m_logger(Acts::getDefaultLogger(name(), level)) { - if (!m_cfg.downstreamReader) { - throw std::invalid_argument("No downstream reader provided!"); + if (!m_cfg.upstreamReader) { + throw std::invalid_argument("No upstream reader provided!"); } // Register write and read handles of the downstream reader - for (auto rh : m_cfg.downstreamReader->readHandles()) { + for (auto rh : m_cfg.upstreamReader->readHandles()) { registerReadHandle(*rh); } - for (auto wh : m_cfg.downstreamReader->writeHandles()) { + for (auto wh : m_cfg.upstreamReader->writeHandles()) { registerWriteHandle(*wh); } // Read the events - auto [ebegin, eend] = m_cfg.downstreamReader->availableEvents(); + auto [ebegin, eend] = m_cfg.upstreamReader->availableEvents(); if (eend - ebegin < m_cfg.bufferSize) { throw std::runtime_error("Reader does not provide enough events"); } @@ -46,7 +46,7 @@ BufferedReader::BufferedReader(const Config &config, Acts::Logging::Level level) ActsExamples::AlgorithmContext ctx(0, i, *board); ACTS_DEBUG("Read event " << i << " into buffer"); - m_cfg.downstreamReader->read(ctx); + m_cfg.upstreamReader->read(ctx); m_buffer.emplace_back(std::move(board)); } @@ -65,7 +65,7 @@ ProcessCode BufferedReader::read(const AlgorithmContext &ctx) { std::uniform_int_distribution dist(0, m_cfg.bufferSize - 1); const auto entry = dist(rng); - m_buffer.at(entry)->shareDataWith(ctx.eventStore); + ctx.eventStore.copyFrom(m_buffer.at(entry)); ACTS_DEBUG("Use buffer entry " << entry << " for event " << ctx.eventNumber); diff --git a/Examples/Framework/src/Framework/WhiteBoard.cpp b/Examples/Framework/src/Framework/WhiteBoard.cpp index c8ccb2a896e..f85ce90ea1f 100644 --- a/Examples/Framework/src/Framework/WhiteBoard.cpp +++ b/Examples/Framework/src/Framework/WhiteBoard.cpp @@ -85,13 +85,14 @@ std::string ActsExamples::WhiteBoard::typeMismatchMessage( boost::core::demangle(act)}; } -void ActsExamples::WhiteBoard::shareDataWith(WhiteBoard &other) const { - for (auto &[key, val] : m_store) { - auto [it, success] = other.m_store.insert({key, val}); +void ActsExamples::WhiteBoard::copyFrom(const WhiteBoard &other) { + for (auto &[key, val] : other.m_store) { + auto [it, success] = m_store.insert({key, val}); if (!success) { - throw std::runtime_error("Cannot share key '" + key + + throw std::runtime_error("Cannot insert key '" + key + "', is already present"); } + ACTS_VERBOSE("Copied key '" << key << "' to whiteboard"); } } diff --git a/Examples/Python/src/Input.cpp b/Examples/Python/src/Input.cpp index 70920152c6f..60e0424b208 100644 --- a/Examples/Python/src/Input.cpp +++ b/Examples/Python/src/Input.cpp @@ -42,7 +42,7 @@ void addInput(Context& ctx) { // Buffered reader ACTS_PYTHON_DECLARE_READER(ActsExamples::BufferedReader, mex, - "BufferedReader", downstreamReader, selectionSeed, + "BufferedReader", upstreamReader, selectionSeed, bufferSize); // ROOT READERS diff --git a/Examples/Python/tests/test_reader.py b/Examples/Python/tests/test_reader.py index edaed74f001..5faf358efd3 100644 --- a/Examples/Python/tests/test_reader.py +++ b/Examples/Python/tests/test_reader.py @@ -471,7 +471,7 @@ def test_buffered_reader(tmp_path, conf_const, ptcl_gun): s2.addReader( acts.examples.BufferedReader( level=acts.logging.WARNING, - downstreamReader=reader, + upstreamReader=reader, bufferSize=eventsInBuffer, ) ) From d8111a4e391bb97443ceaa720c2a2f56750b6abc Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Thu, 16 Jan 2025 12:53:21 +0100 Subject: [PATCH 5/9] update --- .../Framework/include/ActsExamples/Framework/BufferedReader.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp b/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp index 75a884163d4..5f12c5df30a 100644 --- a/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/BufferedReader.hpp @@ -54,7 +54,7 @@ class BufferedReader final : public IReader { } /// Return a event from the buffer - ProcessCode read(const AlgorithmContext& context) override; + ProcessCode read(const AlgorithmContext& ctx) override; /// Fulfill the algorithm interface ProcessCode initialize() override { return ProcessCode::SUCCESS; } From 29560cc388b5fd3b9ae016ba9fc44103a67d1196 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Thu, 16 Jan 2025 12:59:19 +0100 Subject: [PATCH 6/9] update --- Examples/Framework/src/Framework/BufferedReader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Examples/Framework/src/Framework/BufferedReader.cpp b/Examples/Framework/src/Framework/BufferedReader.cpp index 597ff83b7e5..bf46adafcc0 100644 --- a/Examples/Framework/src/Framework/BufferedReader.cpp +++ b/Examples/Framework/src/Framework/BufferedReader.cpp @@ -65,7 +65,7 @@ ProcessCode BufferedReader::read(const AlgorithmContext &ctx) { std::uniform_int_distribution dist(0, m_cfg.bufferSize - 1); const auto entry = dist(rng); - ctx.eventStore.copyFrom(m_buffer.at(entry)); + ctx.eventStore.copyFrom(*m_buffer.at(entry)); ACTS_DEBUG("Use buffer entry " << entry << " for event " << ctx.eventNumber); From af06548840ef499337eff60074ca061e51d0db35 Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Thu, 16 Jan 2025 14:51:22 +0100 Subject: [PATCH 7/9] pauls patch --- .../ActsExamples/Framework/WhiteBoard.hpp | 57 ++++++++----------- .../Framework/src/Framework/WhiteBoard.cpp | 36 ++++++++++-- 2 files changed, 55 insertions(+), 38 deletions(-) diff --git a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp index 84170b44b91..775fd938b9a 100644 --- a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp @@ -53,22 +53,6 @@ class WhiteBoard { void copyFrom(const WhiteBoard& other); private: - /// Store an object on the white board and transfer ownership. - /// - /// @param name Non-empty identifier to store it under - /// @param object Movable reference to the transferable object - /// @throws std::invalid_argument on empty or duplicate name - template - void add(const std::string& name, T&& object); - - /// Get access to a stored object. - /// - /// @param[in] name Identifier for the object - /// @return reference to the stored object - /// @throws std::out_of_range if no object is stored under the requested name - template - const T& get(const std::string& name) const; - private: /// Find similar names for suggestions with levenshtein-distance std::vector similarNames(const std::string_view& name, @@ -88,6 +72,30 @@ class WhiteBoard { const std::type_info& type() const override { return typeid(T); } }; + /// Store an holder on the white board and transfer ownership. + /// + /// @param name Non-empty identifier to store it under + /// @param holder The holder to store + /// @throws std::invalid_argument on empty or duplicate name + void addHolder(const std::string& name, std::shared_ptr holder); + + /// Store an object on the white board and transfer ownership. + /// + /// @param name Non-empty identifier to store it under + /// @param object Movable reference to the transferable object + template + void add(const std::string& name, T&& object) { + addHolder(name, std::make_shared>(std::forward(object))); + } + + /// Get access to a stored object. + /// + /// @param[in] name Identifier for the object + /// @return reference to the stored object + /// @throws std::out_of_range if no object is stored under the requested name + template + const T& get(const std::string& name) const; + std::unique_ptr m_logger; std::unordered_map> m_store; std::unordered_map m_objectAliases; @@ -111,23 +119,6 @@ inline ActsExamples::WhiteBoard::WhiteBoard( std::unordered_map objectAliases) : m_logger(std::move(logger)), m_objectAliases(std::move(objectAliases)) {} -template -inline void ActsExamples::WhiteBoard::add(const std::string& name, T&& object) { - if (name.empty()) { - throw std::invalid_argument("Object can not have an empty name"); - } - if (m_store.contains(name)) { - throw std::invalid_argument("Object '" + name + "' already exists"); - } - auto holder = std::make_shared>(std::forward(object)); - m_store.emplace(name, holder); - ACTS_VERBOSE("Added object '" << name << "' of type " << typeid(T).name()); - if (auto it = m_objectAliases.find(name); it != m_objectAliases.end()) { - m_store[it->second] = holder; - ACTS_VERBOSE("Added alias object '" << it->second << "'"); - } -} - template inline const T& ActsExamples::WhiteBoard::get(const std::string& name) const { ACTS_VERBOSE("Attempt to get object '" << name << "' of type " diff --git a/Examples/Framework/src/Framework/WhiteBoard.cpp b/Examples/Framework/src/Framework/WhiteBoard.cpp index f85ce90ea1f..1a8dab9f25e 100644 --- a/Examples/Framework/src/Framework/WhiteBoard.cpp +++ b/Examples/Framework/src/Framework/WhiteBoard.cpp @@ -67,6 +67,11 @@ std::vector ActsExamples::WhiteBoard::similarNames( names.push_back({d, n}); } } + for (const auto &[from, to] : m_objectAliases) { + if (const auto d = levenshteinDistance(from, name); d < distThreshold) { + names.push_back({d, from}); + } + } std::ranges::sort(names, {}, [](const auto &n) { return n.first; }); @@ -87,12 +92,33 @@ std::string ActsExamples::WhiteBoard::typeMismatchMessage( void ActsExamples::WhiteBoard::copyFrom(const WhiteBoard &other) { for (auto &[key, val] : other.m_store) { - auto [it, success] = m_store.insert({key, val}); + addHolder(key, val); + ACTS_VERBOSE("Copied key '" << key << "' to whiteboard"); + } +} + +void ActsExamples::WhiteBoard::addHolder(const std::string &name, + std::shared_ptr holder) { + if (name.empty()) { + throw std::invalid_argument("Object can not have an empty name"); + } - if (!success) { - throw std::runtime_error("Cannot insert key '" + key + - "', is already present"); + if (holder == nullptr) { + throw std::invalid_argument("Object '" + name + "' is nullptr"); + } + + auto [storeIt, success] = m_store.insert({name, std::move(holder)}); + + if (!success) { + throw std::invalid_argument("Object '" + name + "' already exists"); + } + ACTS_VERBOSE("Added object '" << name << "' of type " + << storeIt->second->type().name()); + + if (success) { + if (auto it = m_objectAliases.find(name); it != m_objectAliases.end()) { + m_store[it->second] = storeIt->second; + ACTS_VERBOSE("Added alias object '" << it->second << "'"); } - ACTS_VERBOSE("Copied key '" << key << "' to whiteboard"); } } From 8040fd3f94e40c50662c6729e0bf1264e4c41dfd Mon Sep 17 00:00:00 2001 From: Benjamin Huth Date: Thu, 16 Jan 2025 14:53:50 +0100 Subject: [PATCH 8/9] remove double private --- Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp index 775fd938b9a..1021f6a32e9 100644 --- a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp @@ -52,7 +52,6 @@ class WhiteBoard { /// the keys in the other whiteboard. void copyFrom(const WhiteBoard& other); - private: private: /// Find similar names for suggestions with levenshtein-distance std::vector similarNames(const std::string_view& name, From 3ca8ff18d2d51dafa6c64de72e10770f2e4a2e87 Mon Sep 17 00:00:00 2001 From: Benjamin Huth <37871400+benjaminhuth@users.noreply.github.com> Date: Fri, 17 Jan 2025 10:24:24 +0100 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Andreas Stefl --- .../Framework/include/ActsExamples/Framework/WhiteBoard.hpp | 2 +- Examples/Framework/src/Framework/BufferedReader.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp index 1021f6a32e9..44cc22ee712 100644 --- a/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/WhiteBoard.hpp @@ -71,7 +71,7 @@ class WhiteBoard { const std::type_info& type() const override { return typeid(T); } }; - /// Store an holder on the white board and transfer ownership. + /// Store a holder on the white board. /// /// @param name Non-empty identifier to store it under /// @param holder The holder to store diff --git a/Examples/Framework/src/Framework/BufferedReader.cpp b/Examples/Framework/src/Framework/BufferedReader.cpp index bf46adafcc0..4431fd99f5d 100644 --- a/Examples/Framework/src/Framework/BufferedReader.cpp +++ b/Examples/Framework/src/Framework/BufferedReader.cpp @@ -23,7 +23,7 @@ BufferedReader::BufferedReader(const Config &config, Acts::Logging::Level level) throw std::invalid_argument("No upstream reader provided!"); } - // Register write and read handles of the downstream reader + // Register write and read handles of the upstream reader for (auto rh : m_cfg.upstreamReader->readHandles()) { registerReadHandle(*rh); }