From 4aa3d3afb6acb0a5a270de4c88d65a4303031053 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Thu, 3 Sep 2026 15:41:07 +0200 Subject: [PATCH 1/3] [hist] Strengthen test RProfile.FillWeight --- hist/histv7/test/hist_profile.cxx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hist/histv7/test/hist_profile.cxx b/hist/histv7/test/hist_profile.cxx index f671367412840..4db6784f1036e 100644 --- a/hist/histv7/test/hist_profile.cxx +++ b/hist/histv7/test/hist_profile.cxx @@ -303,10 +303,12 @@ TEST(RProfile, FillWeight) EXPECT_EQ(profile.GetNEntries(), 2); EXPECT_FLOAT_EQ(profile.GetStats().GetSumW(), 1.7); EXPECT_FLOAT_EQ(profile.GetStats().GetSumW2(), 1.45); - // Cross-checked with TH1 + // Cross-checked with TProfile EXPECT_FLOAT_EQ(profile.ComputeNEffectiveEntries(), 1.9931034); EXPECT_FLOAT_EQ(profile.ComputeMean(0), 9.0294118); EXPECT_FLOAT_EQ(profile.ComputeStdDev(0), 0.49913420); + EXPECT_FLOAT_EQ(profile.ComputeMean(1), 24.058824); + EXPECT_FLOAT_EQ(profile.ComputeStdDev(1), 0.99826840); } TEST(RProfile, FillWeightInvalidNumberOfArguments) From d7c36403ac79d961956594c3583e64e1300b74a0 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Thu, 3 Sep 2026 15:43:26 +0200 Subject: [PATCH 2/3] [hist] Simplify test RProfile.FillForward I extended this test from one to two dimensions in commit 1e27e4c6f5 ("Integrate RHistStats into RProfile"), maybe because I was under the impression that the second dimension has a relation to the additional value argument. This is not needed, since the test already checks all combinations of std::tuple and variadic function, with RWeight and without. --- hist/histv7/test/hist_profile.cxx | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/hist/histv7/test/hist_profile.cxx b/hist/histv7/test/hist_profile.cxx index 4db6784f1036e..2c243223ca52a 100644 --- a/hist/histv7/test/hist_profile.cxx +++ b/hist/histv7/test/hist_profile.cxx @@ -410,23 +410,22 @@ TEST(RProfile, FillExceptionSafety) TEST(RProfile, FillForward) { static constexpr std::size_t Bins = 20; - const RRegularAxis axis(Bins, {0, Bins}); - RProfile profile(axis, axis); + RProfile profile(Bins, {0, Bins}); CopyArgument value(23.0); - std::tuple args(1.5, 2.5); + std::tuple args(1.5); profile.Fill(args, value); profile.Fill(args, value, RWeight(0.5)); EXPECT_EQ(profile.GetNEntries(), 2); - EXPECT_EQ(profile.GetBinContent(1, 2).fSumValues, 34.5); + EXPECT_EQ(profile.GetBinContent(1).fSumValues, 34.5); ASSERT_FALSE(CopyArgument::HasBeenCopied()); - CopyArgument arg1(3.5), arg2(4.5); - profile.Fill(arg1, arg2, value); - profile.Fill(arg1, arg2, value, RWeight(0.5)); + CopyArgument arg(2.5); + profile.Fill(arg, value); + profile.Fill(arg, value, RWeight(0.5)); EXPECT_EQ(profile.GetNEntries(), 4); - EXPECT_EQ(profile.GetBinContent(3, 4).fSumValues, 34.5); + EXPECT_EQ(profile.GetBinContent(2).fSumValues, 34.5); ASSERT_FALSE(CopyArgument::HasBeenCopied()); } From a4948b13fb7e7671587e289f0be09e8d1cfc72bd Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Thu, 3 Sep 2026 15:50:15 +0200 Subject: [PATCH 3/3] [hist] Implement RProfileConcurrentFiller Same concept as RHistConcurrentFiller and RHistFillContext, with one local RHistStats object per RProfileFillContext. The implementation is slightly more complex because of the additional variable. --- hist/histv7/headers.cmake | 2 + hist/histv7/inc/ROOT/RHistEngine.hxx | 28 +- hist/histv7/inc/ROOT/RProfile.hxx | 25 ++ .../inc/ROOT/RProfileConcurrentFiller.hxx | 93 ++++++ hist/histv7/inc/ROOT/RProfileFillContext.hxx | 147 +++++++++ hist/histv7/test/CMakeLists.txt | 1 + hist/histv7/test/hist_profile_concurrent.cxx | 282 ++++++++++++++++++ hist/histv7/test/hist_test.hxx | 4 + 8 files changed, 577 insertions(+), 5 deletions(-) create mode 100644 hist/histv7/inc/ROOT/RProfileConcurrentFiller.hxx create mode 100644 hist/histv7/inc/ROOT/RProfileFillContext.hxx create mode 100644 hist/histv7/test/hist_profile_concurrent.cxx diff --git a/hist/histv7/headers.cmake b/hist/histv7/headers.cmake index a42cb5d002116..27ea41e47231e 100644 --- a/hist/histv7/headers.cmake +++ b/hist/histv7/headers.cmake @@ -15,6 +15,8 @@ set(histv7_headers ROOT/RHistUtils.hxx ROOT/RLinearizedIndex.hxx ROOT/RProfile.hxx + ROOT/RProfileConcurrentFiller.hxx + ROOT/RProfileFillContext.hxx ROOT/RRegularAxis.hxx ROOT/RSliceBinIndexMapper.hxx ROOT/RSliceSpec.hxx diff --git a/hist/histv7/inc/ROOT/RHistEngine.hxx b/hist/histv7/inc/ROOT/RHistEngine.hxx index fec1b25164577..29a4d8f5fdf5f 100644 --- a/hist/histv7/inc/ROOT/RHistEngine.hxx +++ b/hist/histv7/inc/ROOT/RHistEngine.hxx @@ -78,6 +78,7 @@ class RHistEngine final { friend class RHist; friend class RProfile; + friend class RProfileFillContext; /// The axis configuration for this histogram. Relevant methods are forwarded from the public interface. Internal::RAxes fAxes; @@ -565,6 +566,27 @@ public: } } + /// \} + // End the group to ensure that all contained member functions are public. + +private: + // Also used by RProfileFillContext::Fill(const A &...args) - similar to the variadic + // RHistEngine::FillAtomic(const A &...args) below, is has all arguments in the forwarded std::tuple and needs to + // explicitly specify how many of them should be used by RAxes::ComputeGlobalIndexImpl(args). + template + void FillAtomicImpl(const std::tuple &args, const W &weight) + { + RLinearizedIndex index = fAxes.ComputeGlobalIndexImpl(args); + if (index.fValid) { + assert(index.fIndex < fBinContents.size()); + Internal::AtomicAddRelease(&fBinContents[index.fIndex], weight); + } + } + +public: + /// \name Filling + /// \{ + /// Fill an entry into the histogram with a user-defined weight using atomic instructions. /// /// This overload is only available for user-defined bin content types. @@ -587,11 +609,7 @@ public: if (sizeof...(A) != GetNDimensions()) { throw std::invalid_argument("invalid number of arguments to Fill"); } - RLinearizedIndex index = fAxes.ComputeGlobalIndexImpl(args); - if (index.fValid) { - assert(index.fIndex < fBinContents.size()); - Internal::AtomicAddRelease(&fBinContents[index.fIndex], weight); - } + FillAtomicImpl(args, weight); } /// Fill an entry into the histogram using atomic instructions. diff --git a/hist/histv7/inc/ROOT/RProfile.hxx b/hist/histv7/inc/ROOT/RProfile.hxx index a081ec31e2acb..ddf7be5ef7da9 100644 --- a/hist/histv7/inc/ROOT/RProfile.hxx +++ b/hist/histv7/inc/ROOT/RProfile.hxx @@ -56,6 +56,8 @@ ROOT::Experimental::RProfile profile(axes); Feedback is welcome! */ class RProfile final { + friend class RProfileFillContext; + struct RValueWrapper { double fValue; @@ -113,6 +115,29 @@ public: return *this; } + private: + void AtomicAddRelease(double v, double v2, double w, double w2) + { + Internal::AtomicAddRelease(&fSumValues, v); + Internal::AtomicAddRelease(&fSumValues2, v2); + Internal::AtomicAddRelease(&fSum, w); + Internal::AtomicAddRelease(&fSum2, w2); + } + + public: + void AtomicAddRelease(const RValueWrapper &rhs) + { + double v = rhs.fValue; + AtomicAddRelease(v, v * v, 1.0, 1.0); + } + + void AtomicAddRelease(const RValueWeightWrapper &rhs) + { + double v = rhs.fValue; + double w = rhs.fWeight; + AtomicAddRelease(w * v, w * v * v, w, w * w); + } + /// Add another bin content using atomic instructions. /// /// \param[in] rhs another bin content that must not be modified during the operation diff --git a/hist/histv7/inc/ROOT/RProfileConcurrentFiller.hxx b/hist/histv7/inc/ROOT/RProfileConcurrentFiller.hxx new file mode 100644 index 0000000000000..9b59dc7b65590 --- /dev/null +++ b/hist/histv7/inc/ROOT/RProfileConcurrentFiller.hxx @@ -0,0 +1,93 @@ +/// \file +/// \warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +/// Feedback is welcome! + +#ifndef ROOT_RProfileConcurrentFiller +#define ROOT_RProfileConcurrentFiller + +#include "RProfile.hxx" +#include "RProfileFillContext.hxx" + +#include +#include +#include +#include +#include + +namespace ROOT { +namespace Experimental { + +/** +A histogram filler to concurrently fill an RProfile. + +\code +auto profile = std::make_shared(10, std::make_pair(5, 15)); +{ + ROOT::Experimental::RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + context->Fill(8.5, 23.0); +} +// profile->GetBinContent(ROOT::Experimental::RBinIndex(3)) will return the filled bin content +\endcode + +\warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +Feedback is welcome! +*/ +class RProfileConcurrentFiller final { + /// A pointer to the filled profile histogram + std::shared_ptr fProfile; + + /// Mutex to protect access to the list of fill contexts (not for filling itself!) + std::mutex fMutex; + /// The list of fill contexts, for checks during destruction + std::vector> fFillContexts; + +public: + /// Create a filler object. + /// + /// \param[in] hist a pointer to the histogram + explicit RProfileConcurrentFiller(std::shared_ptr profile) : fProfile(profile) + { + if (!profile) { + throw std::invalid_argument("profile must not be nullptr"); + } + } + + RProfileConcurrentFiller(const RProfileConcurrentFiller &) = delete; + RProfileConcurrentFiller(RProfileConcurrentFiller &&) = delete; + RProfileConcurrentFiller &operator=(const RProfileConcurrentFiller &) = delete; + RProfileConcurrentFiller &operator=(RProfileConcurrentFiller &&) = delete; + + ~RProfileConcurrentFiller() + { + for (const auto &context : fFillContexts) { + if (!context.expired()) { + // According to C++ Core Guideline C.36 "A destructor must not fail" and (C.37) "If a destructor tries to + // exit with an exception, it’s a bad design error and the program had better terminate". + std::terminate(); // GCOVR_EXCL_LINE + } + } + } + + const std::shared_ptr &GetProfile() const { return fProfile; } + + /// Create a new context for concurrent filling. + std::shared_ptr CreateFillContext() + { + // Cannot use std::make_shared because the constructor of RProfileFillContext is private. Also it would mean that + // the (direct) memory of all contexts stays around until the vector of weak_ptr's is cleared. + std::shared_ptr context(new RProfileFillContext(*fProfile)); + + { + std::lock_guard g(fMutex); + fFillContexts.push_back(context); + } + + return context; + } +}; + +} // namespace Experimental +} // namespace ROOT + +#endif diff --git a/hist/histv7/inc/ROOT/RProfileFillContext.hxx b/hist/histv7/inc/ROOT/RProfileFillContext.hxx new file mode 100644 index 0000000000000..5d42ea5c4d341 --- /dev/null +++ b/hist/histv7/inc/ROOT/RProfileFillContext.hxx @@ -0,0 +1,147 @@ +/// \file +/// \warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +/// Feedback is welcome! + +#ifndef ROOT_RProfileFillContext +#define ROOT_RProfileFillContext + +#include "RProfile.hxx" +#include "RHistEngine.hxx" +#include "RHistStats.hxx" +#include "RHistUtils.hxx" +#include "RWeight.hxx" + +#include +#include + +namespace ROOT { +namespace Experimental { + +/** +A context to concurrently fill an RProfile. + +\sa RProfileConcurrentFiller + +\warning This is part of the %ROOT 7 prototype! It will change without notice. It might trigger earthquakes. +Feedback is welcome! +*/ +class RProfileFillContext final { + friend class RProfileConcurrentFiller; + +private: + /// A pointer to the filled profile histogram + RProfile *fProfile = nullptr; + + /// Local histogram statistics + RHistStats fStats; + + /// \sa RProfileConcurrentFiller::CreateFillContent() + explicit RProfileFillContext(RProfile &profile) : fProfile(&profile), fStats(profile.GetNDimensions() + 1) + { + // Propagate disabled dimensions to the local histogram statistics object. + const auto &stats = profile.GetStats(); + for (std::size_t i = 0; i < stats.GetNDimensions(); i++) { + if (!stats.IsEnabled(i)) { + fStats.DisableDimension(i); + } + } + } + RProfileFillContext(const RProfileFillContext &) = delete; + RProfileFillContext(RProfileFillContext &&) = default; + RProfileFillContext &operator=(const RProfileFillContext &) = delete; + RProfileFillContext &operator=(RProfileFillContext &&) = default; + +public: + ~RProfileFillContext() { Flush(); } + + /// Fill an entry into the profile histogram. + /// + /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently + /// discarded. + /// + /// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be + /// converted for the axis type at run-time. + /// + /// \param[in] args the arguments for each axis + /// \param[in] v the additional argument + /// \sa RProfile::Fill(const std::tuple &args, const V &value) + template + void Fill(const std::tuple &args, const V &value) + { + RProfile::RValueWrapper wrapper(value); + fProfile->fEngine.FillAtomic(args, wrapper); + // Avoid a second conversion of value, which we already did in wrapper. + fStats.Fill(Internal::AppendReference(args, wrapper.fValue)); + } + + /// Fill an entry into the profile histogram with a weight. + /// + /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently + /// discarded. + /// + /// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be + /// converted for the axis type at run-time. + /// + /// \param[in] args the arguments for each axis + /// \param[in] v the additional argument + /// \param[in] weight the weight for this entry + /// \sa RProfile::Fill(const std::tuple &args, const V &value, RWeight weight) + template + void Fill(const std::tuple &args, const V &value, RWeight weight) + { + RProfile::RValueWeightWrapper wrapper(value, weight.fValue); + fProfile->fEngine.FillAtomic(args, wrapper); + // Avoid a second conversion of value, which we already did in wrapper. + fStats.Fill(Internal::AppendReference(args, wrapper.fValue), weight); + } + + /// Fill an entry into the profile histogram. + /// + /// For weighted filling, pass an RWeight as the last argument. + /// + /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently + /// discarded. + /// + /// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be + /// converted for the axis type at run-time. + /// + /// \param[in] args the arguments for each axis + /// \sa RProfile::Fill(const A &...args) + template + void Fill(const A &...args) + { + static_assert(sizeof...(A) >= 2, "need at least two arguments to Fill"); + if constexpr (sizeof...(A) >= 2) { + auto t = std::forward_as_tuple(args...); + if constexpr (std::is_same_v::type, RWeight>) { + static constexpr std::size_t N = sizeof...(A) - 2; + if (N != fProfile->GetNDimensions()) { + throw std::invalid_argument("invalid number of arguments to Fill"); + } + RWeight weight = std::get(t); + RProfile::RValueWeightWrapper wrapper(std::get(t), weight.fValue); + fProfile->fEngine.FillAtomicImpl(t, wrapper); + } else { + static constexpr std::size_t N = sizeof...(A) - 1; + if (N != fProfile->GetNDimensions()) { + throw std::invalid_argument("invalid number of arguments to Fill"); + } + RProfile::RValueWrapper wrapper(std::get(t)); + fProfile->fEngine.FillAtomicImpl(t, wrapper); + } + fStats.Fill(args...); + } + } + + /// Flush locally accumulated entries to the profile histogram. + void Flush() + { + fProfile->fStats.AddAtomic(fStats); + fStats.Clear(); + } +}; + +} // namespace Experimental +} // namespace ROOT + +#endif diff --git a/hist/histv7/test/CMakeLists.txt b/hist/histv7/test/CMakeLists.txt index eeb9a3463878e..c83ffc6302912 100644 --- a/hist/histv7/test/CMakeLists.txt +++ b/hist/histv7/test/CMakeLists.txt @@ -9,6 +9,7 @@ HIST_ADD_GTEST(hist_engine_atomic hist_engine_atomic.cxx) HIST_ADD_GTEST(hist_hist hist_hist.cxx) HIST_ADD_GTEST(hist_index hist_index.cxx) HIST_ADD_GTEST(hist_profile hist_profile.cxx) +HIST_ADD_GTEST(hist_profile_concurrent hist_profile_concurrent.cxx) HIST_ADD_GTEST(hist_regular hist_regular.cxx) HIST_ADD_GTEST(hist_slice hist_slice.cxx) HIST_ADD_GTEST(hist_stats hist_stats.cxx) diff --git a/hist/histv7/test/hist_profile_concurrent.cxx b/hist/histv7/test/hist_profile_concurrent.cxx new file mode 100644 index 0000000000000..4c8bd7001575b --- /dev/null +++ b/hist/histv7/test/hist_profile_concurrent.cxx @@ -0,0 +1,282 @@ +#include "hist_test.hxx" + +#include +#include +#include +#include +#include + +TEST(RProfileConcurrentFiller, Constructor) +{ + static constexpr std::size_t Bins = 20; + auto profile = std::make_shared(Bins, std::make_pair(0, Bins)); + RProfileConcurrentFiller filler(profile); + + std::shared_ptr profilePtr = filler.GetProfile(); + EXPECT_EQ(profile, profilePtr); + + auto context = filler.CreateFillContext(); + context->Flush(); + + EXPECT_THROW(RProfileConcurrentFiller(nullptr), std::invalid_argument); +} + +TEST(RProfileConcurrentFiller, OldEntries) +{ + static constexpr std::size_t Bins = 20; + auto profile = std::make_shared(Bins, std::make_pair(0, Bins)); + profile->Fill(8.5, 23.0); + ASSERT_EQ(profile->GetNEntries(), 1); + ASSERT_EQ(profile->GetBinContent(8).fSum, 1.0); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + context->Flush(); + } + + EXPECT_EQ(profile->GetNEntries(), 1); + EXPECT_EQ(profile->GetBinContent(8).fSum, 1.0); +} + +TEST(RProfileFillContext, Fill) +{ + static constexpr std::size_t Bins = 20; + auto profile = std::make_shared(Bins, std::make_pair(0, Bins)); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + context->Fill(8.5, 23.0); + context->Fill(std::make_tuple(9.5), 25.0); + } + + auto &bin8 = profile->GetBinContent(RBinIndex(8)); + EXPECT_EQ(bin8.fSumValues, 23.0); + EXPECT_EQ(bin8.fSumValues2, 529.0); + EXPECT_EQ(bin8.fSum, 1.0); + EXPECT_EQ(bin8.fSum2, 1.0); + std::array indices = {9}; + auto &bin9 = profile->GetBinContent(indices); + EXPECT_EQ(bin9.fSumValues, 25.0); + EXPECT_EQ(bin9.fSumValues2, 625.0); + EXPECT_EQ(bin9.fSum, 1.0); + EXPECT_EQ(bin9.fSum2, 1.0); + + EXPECT_EQ(profile->GetNEntries(), 2); + EXPECT_FLOAT_EQ(profile->ComputeNEffectiveEntries(), 2); + EXPECT_FLOAT_EQ(profile->ComputeMean(0), 9); + EXPECT_FLOAT_EQ(profile->ComputeStdDev(0), 0.5); + EXPECT_FLOAT_EQ(profile->ComputeMean(1), 24.0); + EXPECT_FLOAT_EQ(profile->ComputeStdDev(1), 1.0); +} + +TEST(RProfileFillContext, StressFill) +{ + static constexpr std::size_t NThreads = 4; + static constexpr std::size_t NFillsPerThread = 10000; + static constexpr std::size_t FlushEveryNFills = 500; + static constexpr std::size_t NFills = NThreads * NFillsPerThread; + + // Fill a single bin, to maximize contention. + auto profile = std::make_shared(1, std::make_pair(0, 1)); + { + RProfileConcurrentFiller filler(profile); + StressInParallel(NThreads, [&] { + auto context = filler.CreateFillContext(); + for (std::size_t i = 0; i < NFillsPerThread; i++) { + context->Fill(0.5, 1.5); + if (i % FlushEveryNFills == 0) { + context->Flush(); + } + } + }); + } + + EXPECT_EQ(profile->GetBinContent(0).fSum, NFills); + EXPECT_EQ(profile->GetNEntries(), NFills); + EXPECT_FLOAT_EQ(profile->ComputeNEffectiveEntries(), NFills); + EXPECT_FLOAT_EQ(profile->ComputeMean(0), 0.5); + EXPECT_FLOAT_EQ(profile->ComputeMean(1), 1.5); +} + +TEST(RProfileFillContext, FillWeight) +{ + static constexpr std::size_t Bins = 20; + auto profile = std::make_shared(Bins, std::make_pair(0, Bins)); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + context->Fill(8.5, 23.0, RWeight(0.8)); + context->Fill(std::make_tuple(9.5), 25.0, RWeight(0.9)); + } + + auto &bin8 = profile->GetBinContent(RBinIndex(8)); + EXPECT_FLOAT_EQ(bin8.fSumValues, 18.4); + EXPECT_FLOAT_EQ(bin8.fSumValues2, 423.2); + EXPECT_FLOAT_EQ(bin8.fSum, 0.8); + EXPECT_FLOAT_EQ(bin8.fSum2, 0.64); + std::array indices = {9}; + auto &bin9 = profile->GetBinContent(indices); + EXPECT_FLOAT_EQ(bin9.fSumValues, 22.5); + EXPECT_FLOAT_EQ(bin9.fSumValues2, 562.5); + EXPECT_FLOAT_EQ(bin9.fSum, 0.9); + EXPECT_FLOAT_EQ(bin9.fSum2, 0.81); + + EXPECT_EQ(profile->GetNEntries(), 2); + EXPECT_FLOAT_EQ(profile->GetStats().GetSumW(), 1.7); + EXPECT_FLOAT_EQ(profile->GetStats().GetSumW2(), 1.45); + // Cross-checked with TProfile + EXPECT_FLOAT_EQ(profile->ComputeNEffectiveEntries(), 1.9931034); + EXPECT_FLOAT_EQ(profile->ComputeMean(0), 9.0294118); + EXPECT_FLOAT_EQ(profile->ComputeStdDev(0), 0.49913420); + EXPECT_FLOAT_EQ(profile->ComputeMean(1), 24.058824); + EXPECT_FLOAT_EQ(profile->ComputeStdDev(1), 0.99826840); +} + +TEST(RProfileFillContext, StressFillWeight) +{ + static constexpr std::size_t NThreads = 4; + static constexpr std::size_t NFillsPerThread = 10000; + static constexpr std::size_t FlushEveryNFills = 500; + static constexpr std::size_t NFills = NThreads * NFillsPerThread; + static constexpr double Weight = 0.5; + + // Fill a single bin, to maximize contention. + auto profile = std::make_shared(1, std::make_pair(0, 1)); + { + RProfileConcurrentFiller filler(profile); + StressInParallel(NThreads, [&] { + auto context = filler.CreateFillContext(); + for (std::size_t i = 0; i < NFillsPerThread; i++) { + context->Fill(0.5, 1.5, RWeight(Weight)); + if (i % FlushEveryNFills == 0) { + context->Flush(); + } + } + }); + } + + EXPECT_EQ(profile->GetBinContent(0).fSum, NFills * Weight); + EXPECT_EQ(profile->GetNEntries(), NFills); + EXPECT_FLOAT_EQ(profile->ComputeNEffectiveEntries(), NFills); + EXPECT_FLOAT_EQ(profile->ComputeMean(0), 0.5); + EXPECT_FLOAT_EQ(profile->ComputeMean(1), 1.5); +} + +TEST(RProfileFillContext, FillCategorical) +{ + const std::vector categories = {"a", "b", "c"}; + const RCategoricalAxis axis(categories); + const std::vector axes = {axis}; + auto profile = std::make_shared(axes); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + context->Fill("b", 23.0); + context->Fill(std::make_tuple("c"), 25.0); + } + + auto &bin1 = profile->GetBinContent(RBinIndex(1)); + EXPECT_EQ(bin1.fSumValues, 23.0); + EXPECT_EQ(bin1.fSumValues2, 529.0); + EXPECT_EQ(bin1.fSum, 1.0); + EXPECT_EQ(bin1.fSum2, 1.0); + std::array indices = {2}; + auto &bin2 = profile->GetBinContent(indices); + EXPECT_EQ(bin2.fSumValues, 25.0); + EXPECT_EQ(bin2.fSumValues2, 625.0); + EXPECT_EQ(bin2.fSum, 1.0); + EXPECT_EQ(bin2.fSum2, 1.0); + + EXPECT_EQ(profile->GetNEntries(), 2); + EXPECT_FLOAT_EQ(profile->ComputeNEffectiveEntries(), 2); +} + +TEST(RProfileFillContext, FillCategoricalWeight) +{ + const std::vector categories = {"a", "b", "c"}; + const RCategoricalAxis axis(categories); + const std::vector axes = {axis}; + auto profile = std::make_shared(axes); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + context->Fill("b", 23.0, RWeight(0.8)); + context->Fill(std::make_tuple("c"), 25.0, RWeight(0.9)); + } + + auto &bin1 = profile->GetBinContent(RBinIndex(1)); + EXPECT_FLOAT_EQ(bin1.fSumValues, 18.4); + EXPECT_FLOAT_EQ(bin1.fSumValues2, 423.2); + EXPECT_FLOAT_EQ(bin1.fSum, 0.8); + EXPECT_FLOAT_EQ(bin1.fSum2, 0.64); + std::array indices = {2}; + auto &bin2 = profile->GetBinContent(indices); + EXPECT_FLOAT_EQ(bin2.fSumValues, 22.5); + EXPECT_FLOAT_EQ(bin2.fSumValues2, 562.5); + EXPECT_FLOAT_EQ(bin2.fSum, 0.9); + EXPECT_FLOAT_EQ(bin2.fSum2, 0.81); + + EXPECT_EQ(profile->GetNEntries(), 2); + EXPECT_FLOAT_EQ(profile->GetStats().GetSumW(), 1.7); + EXPECT_FLOAT_EQ(profile->GetStats().GetSumW2(), 1.45); + // Cross-checked with TH1 + EXPECT_FLOAT_EQ(profile->ComputeNEffectiveEntries(), 1.9931034); +} + +TEST(RProfileFillContext, FillForward) +{ + static constexpr std::size_t Bins = 20; + auto profile = std::make_shared(Bins, std::make_pair(0, Bins)); + CopyArgument value(23.0); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + std::tuple args(1.5); + context->Fill(args, value); + context->Fill(args, value, RWeight(0.5)); + } + EXPECT_EQ(profile->GetNEntries(), 2); + EXPECT_EQ(profile->GetBinContent(1).fSumValues, 34.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + CopyArgument arg(2.5); + context->Fill(arg, value); + context->Fill(arg, value, RWeight(0.5)); + } + EXPECT_EQ(profile->GetNEntries(), 4); + EXPECT_EQ(profile->GetBinContent(2).fSumValues, 34.5); + + ASSERT_FALSE(CopyArgument::HasBeenCopied()); +} + +TEST(RProfileFillContext, Flush) +{ + static constexpr std::size_t Bins = 20; + auto profile = std::make_shared(Bins, std::make_pair(0, Bins)); + + { + RProfileConcurrentFiller filler(profile); + auto context = filler.CreateFillContext(); + context->Fill(8.5, 23.0); + // Flushing multiple times, explicitly and implicitly (in the destructor) should only add the entries once. + context->Flush(); + context->Flush(); + } + + EXPECT_EQ(profile->GetNEntries(), 1); + auto &bin8 = profile->GetBinContent(RBinIndex(8)); + EXPECT_EQ(bin8.fSumValues, 23.0); + EXPECT_EQ(bin8.fSumValues2, 529.0); + EXPECT_EQ(bin8.fSum, 1.0); + EXPECT_EQ(bin8.fSum2, 1.0); +} diff --git a/hist/histv7/test/hist_test.hxx b/hist/histv7/test/hist_test.hxx index 90f97424cd6d4..61b64e712abf6 100644 --- a/hist/histv7/test/hist_test.hxx +++ b/hist/histv7/test/hist_test.hxx @@ -15,6 +15,8 @@ #include #include #include +#include +#include #include #include #include @@ -33,6 +35,8 @@ using ROOT::Experimental::RHistConcurrentFiller; using ROOT::Experimental::RHistEngine; using ROOT::Experimental::RHistStats; using ROOT::Experimental::RProfile; +using ROOT::Experimental::RProfileConcurrentFiller; +using ROOT::Experimental::RProfileFillContext; using ROOT::Experimental::RRegularAxis; using ROOT::Experimental::RSliceSpec; using ROOT::Experimental::RVariableBinAxis;