From d9c62bb2b8ad500895f2f30dc2f2c3366b6510e5 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 12 Nov 2024 08:55:34 +0100 Subject: [PATCH 1/8] refactor: Stable RNG for smeared digitization in Examples --- .../src/DigitizationAlgorithm.cpp | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp index 72f3905ab57..b053346b585 100644 --- a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp +++ b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp @@ -14,24 +14,18 @@ #include "Acts/Geometry/TrackingGeometry.hpp" #include "Acts/Surfaces/Surface.hpp" #include "Acts/Utilities/BinUtility.hpp" -#include "Acts/Utilities/Result.hpp" #include "ActsExamples/Digitization/ModuleClusters.hpp" #include "ActsExamples/EventData/GeometryContainers.hpp" #include "ActsExamples/EventData/Index.hpp" -#include "ActsExamples/EventData/SimHit.hpp" #include "ActsExamples/Framework/AlgorithmContext.hpp" -#include "ActsExamples/Utilities/GroupBy.hpp" #include "ActsExamples/Utilities/Range.hpp" #include "ActsFatras/EventData/Barcode.hpp" #include "ActsFatras/EventData/Hit.hpp" #include #include -#include -#include #include #include -#include #include #include #include @@ -156,7 +150,7 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( measurementSimHitsMap.reserve(simHits.size()); // Setup random number generator - auto rng = m_cfg.randomNumbers->spawnGenerator(ctx); + auto eventSeed = m_cfg.randomNumbers->generateSeed(ctx); // Some statistics std::size_t skippedHits = 0; @@ -194,6 +188,19 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( ACTS_VERBOSE("Digitizer found for module " << moduleGeoId); } + auto moduleSeed = eventSeed + moduleGeoId.value(); + RandomEngine rng(moduleSeed); + + // Sort simhits by particle id to make sure we can iterate over them in + // a deterministic order. + std::vector sortedSimHitOrdering(moduleSimHits.size()); + std::iota(sortedSimHitOrdering.begin(), sortedSimHitOrdering.end(), 0); + std::sort(sortedSimHitOrdering.begin(), sortedSimHitOrdering.end(), + [&](std::size_t i, std::size_t j) { + return (moduleSimHits.begin() + i)->particleId() < + (moduleSimHits.begin() + j)->particleId(); + }); + // Run the digitizer. Iterate over the hits for this surface inside the // visitor so we do not need to lookup the variant object per-hit. std::visit( @@ -202,9 +209,8 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( digitizer.geometric.segmentation, digitizer.geometric.indices, m_cfg.doMerge, m_cfg.mergeNsigma, m_cfg.mergeCommonCorner); - for (auto h = moduleSimHits.begin(); h != moduleSimHits.end(); ++h) { - const auto& simHit = *h; - const auto simHitIdx = simHits.index_of(h); + for (std::size_t simHitIdx : sortedSimHitOrdering) { + const auto& simHit = *(moduleSimHits.begin() + simHitIdx); DigitizedParameters dParameters; From 79443e7bd30a12273417c7760629c8afba0a9efd Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 12 Nov 2024 10:29:40 +0100 Subject: [PATCH 2/8] simplify --- .../src/DigitizationAlgorithm.cpp | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp index b053346b585..a5ee376efaa 100644 --- a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp +++ b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp @@ -18,6 +18,7 @@ #include "ActsExamples/EventData/GeometryContainers.hpp" #include "ActsExamples/EventData/Index.hpp" #include "ActsExamples/Framework/AlgorithmContext.hpp" +#include "ActsExamples/Framework/RandomNumbers.hpp" #include "ActsExamples/Utilities/Range.hpp" #include "ActsFatras/EventData/Barcode.hpp" #include "ActsFatras/EventData/Hit.hpp" @@ -149,7 +150,6 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( measurementParticlesMap.reserve(simHits.size()); measurementSimHitsMap.reserve(simHits.size()); - // Setup random number generator auto eventSeed = m_cfg.randomNumbers->generateSeed(ctx); // Some statistics @@ -188,19 +188,6 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( ACTS_VERBOSE("Digitizer found for module " << moduleGeoId); } - auto moduleSeed = eventSeed + moduleGeoId.value(); - RandomEngine rng(moduleSeed); - - // Sort simhits by particle id to make sure we can iterate over them in - // a deterministic order. - std::vector sortedSimHitOrdering(moduleSimHits.size()); - std::iota(sortedSimHitOrdering.begin(), sortedSimHitOrdering.end(), 0); - std::sort(sortedSimHitOrdering.begin(), sortedSimHitOrdering.end(), - [&](std::size_t i, std::size_t j) { - return (moduleSimHits.begin() + i)->particleId() < - (moduleSimHits.begin() + j)->particleId(); - }); - // Run the digitizer. Iterate over the hits for this surface inside the // visitor so we do not need to lookup the variant object per-hit. std::visit( @@ -209,8 +196,13 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( digitizer.geometric.segmentation, digitizer.geometric.indices, m_cfg.doMerge, m_cfg.mergeNsigma, m_cfg.mergeCommonCorner); - for (std::size_t simHitIdx : sortedSimHitOrdering) { - const auto& simHit = *(moduleSimHits.begin() + simHitIdx); + for (auto h = moduleSimHits.begin(); h != moduleSimHits.end(); ++h) { + const auto& simHit = *h; + const auto simHitIdx = simHits.index_of(h); + + auto particleSeed = + eventSeed + simHit.particleId().value() + simHit.index(); + RandomEngine rng(particleSeed); DigitizedParameters dParameters; From 72aa801e8f42c6dcfbb7c8b727ae3594f763477b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 12 Nov 2024 10:38:46 +0100 Subject: [PATCH 3/8] one step further --- .../src/DigitizationAlgorithm.cpp | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp index a5ee376efaa..a7b9e3a5f8a 100644 --- a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp +++ b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp @@ -17,6 +17,7 @@ #include "ActsExamples/Digitization/ModuleClusters.hpp" #include "ActsExamples/EventData/GeometryContainers.hpp" #include "ActsExamples/EventData/Index.hpp" +#include "ActsExamples/EventData/SimParticle.hpp" #include "ActsExamples/Framework/AlgorithmContext.hpp" #include "ActsExamples/Framework/RandomNumbers.hpp" #include "ActsExamples/Utilities/Range.hpp" @@ -159,6 +160,10 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( // Thus we need to store the cell data from the simulation. CellsMap cellsMap; + // Count the number of hits per particle and module. Cleared before each + // module. + std::map particleOnModuleHitCount; + ACTS_DEBUG("Starting loop over modules ..."); for (const auto& simHitsGroup : groupByModule(simHits)) { // Manual pair unpacking instead of using @@ -188,6 +193,8 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( ACTS_VERBOSE("Digitizer found for module " << moduleGeoId); } + particleOnModuleHitCount.clear(); + // Run the digitizer. Iterate over the hits for this surface inside the // visitor so we do not need to lookup the variant object per-hit. std::visit( @@ -200,9 +207,18 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( const auto& simHit = *h; const auto simHitIdx = simHits.index_of(h); - auto particleSeed = - eventSeed + simHit.particleId().value() + simHit.index(); - RandomEngine rng(particleSeed); + const auto hitIndex = + particleOnModuleHitCount[simHit.particleId()]++; + + if (hitIndex > 0) { + ACTS_DEBUG("more than one hit for particle " + << simHit.particleId() << " on module " << moduleGeoId + << " - still forwarding it to the digitizer"); + } + + auto hitSeed = eventSeed + moduleGeoId.value() + + simHit.particleId().value() + hitIndex; + RandomEngine rng(hitSeed); DigitizedParameters dParameters; From 31854101347eb338ee60757fa6809ae52fd630bd Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 13 Nov 2024 09:52:12 +0100 Subject: [PATCH 4/8] use 32 bit for seed --- .../src/DigitizationAlgorithm.cpp | 6 +++--- .../ActsExamples/Framework/RandomNumbers.hpp | 17 +++++---------- .../Framework/src/Framework/RandomNumbers.cpp | 21 ++++++++----------- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp index a7b9e3a5f8a..603e5bb7d82 100644 --- a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp +++ b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp @@ -151,7 +151,7 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( measurementParticlesMap.reserve(simHits.size()); measurementSimHitsMap.reserve(simHits.size()); - auto eventSeed = m_cfg.randomNumbers->generateSeed(ctx); + RandomSeed eventSeed = m_cfg.randomNumbers->generateSeed(ctx); // Some statistics std::size_t skippedHits = 0; @@ -216,8 +216,8 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( << " - still forwarding it to the digitizer"); } - auto hitSeed = eventSeed + moduleGeoId.value() + - simHit.particleId().value() + hitIndex; + RandomSeed hitSeed = eventSeed + moduleGeoId.value() + + simHit.particleId().value() + hitIndex; RandomEngine rng(hitSeed); DigitizedParameters dParameters; diff --git a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp index 6f9f9abbcd6..a94bac778f6 100644 --- a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp @@ -6,18 +6,8 @@ // 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/. -// -// RandomNumbers.hpp -// ActsExamples -// -// Created by Andreas Salzburger on 17/05/16. -// -// - #pragma once -#include "ActsExamples/Framework/AlgorithmContext.hpp" - #include #include @@ -27,6 +17,9 @@ struct AlgorithmContext; /// The random number generator used in the framework. using RandomEngine = std::mt19937; ///< Mersenne Twister +/// The seed type used in the framework. +using RandomSeed = std::uint32_t; + /// Provide event and algorithm specific random number generator.s /// /// This provides local random number generators, allowing for @@ -41,7 +34,7 @@ using RandomEngine = std::mt19937; ///< Mersenne Twister class RandomNumbers { public: struct Config { - std::uint64_t seed = 1234567890u; ///< random seed + RandomSeed seed = 1234567890u; ///< random seed }; explicit RandomNumbers(const Config& cfg); @@ -59,7 +52,7 @@ class RandomNumbers { /// /// This should only be used in special cases e.g. where a custom /// random engine is used and `spawnGenerator` can not be used. - std::uint64_t generateSeed(const AlgorithmContext& context) const; + RandomSeed generateSeed(const AlgorithmContext& context) const; private: Config m_cfg; diff --git a/Examples/Framework/src/Framework/RandomNumbers.cpp b/Examples/Framework/src/Framework/RandomNumbers.cpp index 5fd0cdcf7d7..558adbb8b3d 100644 --- a/Examples/Framework/src/Framework/RandomNumbers.cpp +++ b/Examples/Framework/src/Framework/RandomNumbers.cpp @@ -6,26 +6,23 @@ // 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/. -// -// RandomNumbers.cpp -// ActsExamples -// -// Created by Andreas Salzburger on 17/05/16. -// -// - #include "ActsExamples/Framework/RandomNumbers.hpp" #include "ActsExamples/Framework/AlgorithmContext.hpp" -ActsExamples::RandomNumbers::RandomNumbers(const Config& cfg) : m_cfg(cfg) {} +#include -ActsExamples::RandomEngine ActsExamples::RandomNumbers::spawnGenerator( +namespace ActsExamples { + +RandomNumbers::RandomNumbers(const Config& cfg) : m_cfg(cfg) {} + +RandomEngine RandomNumbers::spawnGenerator( const AlgorithmContext& context) const { return RandomEngine(generateSeed(context)); } -std::uint64_t ActsExamples::RandomNumbers::generateSeed( - const AlgorithmContext& context) const { +RandomSeed RandomNumbers::generateSeed(const AlgorithmContext& context) const { return m_cfg.seed + context.eventNumber; } + +} // namespace ActsExamples From c46202ad206b36d889244dbd8c4f4dfe942cc047 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 13 Nov 2024 10:14:58 +0100 Subject: [PATCH 5/8] use 64 bit seed and RNG --- .../Algorithms/Digitization/src/DigitizationAlgorithm.cpp | 2 +- .../include/ActsExamples/Framework/RandomNumbers.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp index 603e5bb7d82..a0529653aff 100644 --- a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp +++ b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp @@ -162,7 +162,7 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( // Count the number of hits per particle and module. Cleared before each // module. - std::map particleOnModuleHitCount; + std::map particleOnModuleHitCount; ACTS_DEBUG("Starting loop over modules ..."); for (const auto& simHitsGroup : groupByModule(simHits)) { diff --git a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp index a94bac778f6..788f67f166f 100644 --- a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp @@ -15,10 +15,10 @@ namespace ActsExamples { struct AlgorithmContext; /// The random number generator used in the framework. -using RandomEngine = std::mt19937; ///< Mersenne Twister +using RandomEngine = std::mt19937_64; ///< Mersenne Twister /// The seed type used in the framework. -using RandomSeed = std::uint32_t; +using RandomSeed = std::uint64_t; /// Provide event and algorithm specific random number generator.s /// From 25d184cb99c93bd0eec61febf8c690e446cb3e95 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 14 Nov 2024 11:50:49 +0100 Subject: [PATCH 6/8] fix silly test --- .../UnitTests/Examples/Io/Json/JsonDigitizationConfigTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/UnitTests/Examples/Io/Json/JsonDigitizationConfigTests.cpp b/Tests/UnitTests/Examples/Io/Json/JsonDigitizationConfigTests.cpp index f7c1db0f496..f316953ba32 100644 --- a/Tests/UnitTests/Examples/Io/Json/JsonDigitizationConfigTests.cpp +++ b/Tests/UnitTests/Examples/Io/Json/JsonDigitizationConfigTests.cpp @@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE(GaussianSmearing) { if (s.forcePositive[i]) { ref = std::abs(ref); } - CHECK_CLOSE_REL(par[i], ref, 0.15); + CHECK_CLOSE_ABS(par[i], ref, 0.15); } } From 8e1be8269edd141502132e22045cd13ffeba1061 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 15 Nov 2024 09:18:21 +0100 Subject: [PATCH 7/8] fix and fold ids to 32 bit --- .../Digitization/src/DigitizationAlgorithm.cpp | 9 +++++++-- .../include/ActsExamples/Framework/RandomNumbers.hpp | 3 --- Examples/Framework/src/Framework/RandomNumbers.cpp | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp index a0529653aff..ce74a6cc81c 100644 --- a/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp +++ b/Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp @@ -216,8 +216,13 @@ ActsExamples::ProcessCode ActsExamples::DigitizationAlgorithm::execute( << " - still forwarding it to the digitizer"); } - RandomSeed hitSeed = eventSeed + moduleGeoId.value() + - simHit.particleId().value() + hitIndex; + RandomSeed moduleSeed = (moduleGeoId.value() & 0xFFFFFFFF) ^ + ((moduleGeoId.value() >> 32) & 0xFFFFFFFF); + RandomSeed particleSeed = + (simHit.particleId().value() & 0xFFFFFFFF) ^ + ((simHit.particleId().value() >> 32) & 0xFFFFFFFF); + RandomSeed hitSeed = + eventSeed + moduleSeed + particleSeed + hitIndex; RandomEngine rng(hitSeed); DigitizedParameters dParameters; diff --git a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp index 27bd4b61bfb..4e168e72efc 100644 --- a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp @@ -17,9 +17,6 @@ struct AlgorithmContext; /// The random number generator used in the framework. using RandomEngine = std::mt19937_64; ///< Mersenne Twister -/// The seed type used in the framework. -using RandomSeed = std::uint64_t; - /// The seed type used in the framework. using RandomSeed = std::uint32_t; diff --git a/Examples/Framework/src/Framework/RandomNumbers.cpp b/Examples/Framework/src/Framework/RandomNumbers.cpp index 4313a5cce77..6b7fe859e94 100644 --- a/Examples/Framework/src/Framework/RandomNumbers.cpp +++ b/Examples/Framework/src/Framework/RandomNumbers.cpp @@ -20,7 +20,7 @@ RandomEngine RandomNumbers::spawnGenerator( } RandomSeed RandomNumbers::generateSeed(const AlgorithmContext& context) const { - return m_cfg.seed + context.eventNumber; + return m_cfg.seed + static_cast(context.eventNumber); } } // namespace ActsExamples From b29988bfbf4e9462aff1e2753d480f992f14f1e2 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 15 Nov 2024 10:16:06 +0100 Subject: [PATCH 8/8] go back to 32 bit RNG --- .../Framework/include/ActsExamples/Framework/RandomNumbers.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp index 4e168e72efc..a94bac778f6 100644 --- a/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp +++ b/Examples/Framework/include/ActsExamples/Framework/RandomNumbers.hpp @@ -15,7 +15,7 @@ namespace ActsExamples { struct AlgorithmContext; /// The random number generator used in the framework. -using RandomEngine = std::mt19937_64; ///< Mersenne Twister +using RandomEngine = std::mt19937; ///< Mersenne Twister /// The seed type used in the framework. using RandomSeed = std::uint32_t;