From f7271a8445e77028fa2381f86b07b67a1ffec94d Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Thu, 30 Oct 2025 13:38:25 +0100 Subject: [PATCH] [hist] Implement RHistEngine::SetBinContent This needs one indirection to construct the std::array of indices with the right (number of) elements. --- hist/histv7/doc/DesignImplementation.md | 7 +-- hist/histv7/inc/ROOT/RHistEngine.hxx | 72 +++++++++++++++++++++++++ hist/histv7/test/hist_engine.cxx | 49 +++++++++++++++++ hist/histv7/test/hist_user.cxx | 22 ++++++++ 4 files changed, 147 insertions(+), 3 deletions(-) diff --git a/hist/histv7/doc/DesignImplementation.md b/hist/histv7/doc/DesignImplementation.md index f0e3407efb58d..83e886143581e 100644 --- a/hist/histv7/doc/DesignImplementation.md +++ b/hist/histv7/doc/DesignImplementation.md @@ -58,11 +58,12 @@ Special arguments are passed last. Examples include ```cpp template void Fill(const std::tuple &args, RWeight w); -template void SetBinContent(const std::array &indices, const BinContentType &content); +template void SetBinContent(const std::array &indices, const V &value); ``` -The same works for the variadic function templates that will check the type of the last argument. +Note that we accept mandatory arguments with a template type as well to allow automatic conversion. -For profiles, we accept the value with a template type as well to allow automatic conversion to `double`, for example from `int`. +Variadic function templates receive all arguments in a single function parameter pack. +For optional arguments, the function will check the type of the last argument to determine if it was passed. ## Miscellaneous diff --git a/hist/histv7/inc/ROOT/RHistEngine.hxx b/hist/histv7/inc/ROOT/RHistEngine.hxx index f60485e061d65..0585dd3be67e4 100644 --- a/hist/histv7/inc/ROOT/RHistEngine.hxx +++ b/hist/histv7/inc/ROOT/RHistEngine.hxx @@ -170,6 +170,78 @@ public: return GetBinContent(indices); } + /// Set the content of a single bin. + /// + /// \code + /// ROOT::Experimental::RHistEngine hist({/* two dimensions */}); + /// std::array indices = {3, 5}; + /// int value = /* ... */; + /// hist.SetBinContent(indices, value); + /// \endcode + /// + /// \note Compared to TH1 conventions, the first normal bin has index 0 and underflow and overflow bins are special + /// values. See also the class documentation of RBinIndex. + /// + /// Throws an exception if the number of indices does not match the axis configuration or the bin is not found. + /// + /// \param[in] indices the array of indices for each axis + /// \param[in] value the new value of the bin content + /// \par See also + /// the \ref SetBinContent(const A &... args) const "variadic function template overload" accepting arguments + /// directly + template + void SetBinContent(const std::array &indices, const V &value) + { + // We could rely on RAxes::ComputeGlobalIndex to check the number of arguments, but its exception message might + // be confusing for users. + if (N != GetNDimensions()) { + throw std::invalid_argument("invalid number of indices passed to SetBinContent"); + } + RLinearizedIndex index = fAxes.ComputeGlobalIndex(indices); + if (!index.fValid) { + throw std::invalid_argument("bin not found in SetBinContent"); + } + assert(index.fIndex < fBinContents.size()); + // To allow conversion, we have to accept value with a template type V to capture any argument. Otherwise it would + // select the variadic function template... + fBinContents[index.fIndex] = value; + } + +private: + template + void SetBinContentImpl(const std::tuple &args, std::index_sequence) + { + std::array indices{std::get(args)...}; + SetBinContent(indices, std::get(args)); + } + +public: + /// Set the content of a single bin. + /// + /// \code + /// ROOT::Experimental::RHistEngine hist({/* two dimensions */}); + /// int value = /* ... */; + /// hist.SetBinContent(ROOT::Experimental::RBinIndex(3), ROOT::Experimental::RBinIndex(5), value); + /// // ... or construct the RBinIndex arguments implicitly from integers: + /// hist.SetBinContent(3, 5, value); + /// \endcode + /// + /// \note Compared to TH1 conventions, the first normal bin has index 0 and underflow and overflow bins are special + /// values. See also the class documentation of RBinIndex. + /// + /// Throws an exception if the number of arguments does not match the axis configuration or the bin is not found. + /// + /// \param[in] args the arguments for each axis and the new value of the bin content + /// \par See also + /// the \ref SetBinContent(const std::array &indices, const V &value) const "function overload" + /// accepting `std::array` + template + void SetBinContent(const A &...args) + { + auto t = std::forward_as_tuple(args...); + SetBinContentImpl(t, std::make_index_sequence()); + } + /// Add all bin contents of another histogram. /// /// Throws an exception if the axes configurations are not identical. diff --git a/hist/histv7/test/hist_engine.cxx b/hist/histv7/test/hist_engine.cxx index decb551f6f550..2e19ef09dc914 100644 --- a/hist/histv7/test/hist_engine.cxx +++ b/hist/histv7/test/hist_engine.cxx @@ -71,6 +71,55 @@ TEST(RHistEngine, GetBinContentNotFound) EXPECT_THROW(engine.GetBinContent(Bins), std::invalid_argument); } +TEST(RHistEngine, SetBinContent) +{ + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engine({axis}); + + const RBinIndex index(7); + engine.SetBinContent(index, 42); + EXPECT_EQ(engine.GetBinContent(index), 42); + + const std::array indices = {index}; + engine.SetBinContent(indices, 43); + EXPECT_EQ(engine.GetBinContent(indices), 43); + + // This also works if the value must be converted to the bin content type. + RHistEngine engineF({axis}); + engineF.SetBinContent(index, 42); + EXPECT_EQ(engineF.GetBinContent(index), 42); + + engineF.SetBinContent(indices, 43); + EXPECT_EQ(engineF.GetBinContent(indices), 43); +} + +TEST(RHistEngine, SetBinContentInvalidNumberOfArguments) +{ + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engine1({axis}); + ASSERT_EQ(engine1.GetNDimensions(), 1); + RHistEngine engine2({axis, axis}); + ASSERT_EQ(engine2.GetNDimensions(), 2); + + EXPECT_NO_THROW(engine1.SetBinContent(1, 0)); + EXPECT_THROW(engine1.SetBinContent(1, 2, 0), std::invalid_argument); + + EXPECT_THROW(engine2.SetBinContent(1, 0), std::invalid_argument); + EXPECT_NO_THROW(engine2.SetBinContent(1, 2, 0)); + EXPECT_THROW(engine2.SetBinContent(1, 2, 3, 0), std::invalid_argument); +} + +TEST(RHistEngine, SetBinContentNotFound) +{ + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engine({axis}); + + EXPECT_THROW(engine.SetBinContent(Bins, 0), std::invalid_argument); +} + TEST(RHistEngine, Add) { static constexpr std::size_t Bins = 20; diff --git a/hist/histv7/test/hist_user.cxx b/hist/histv7/test/hist_user.cxx index cb893b46520ab..bc18a44659f31 100644 --- a/hist/histv7/test/hist_user.cxx +++ b/hist/histv7/test/hist_user.cxx @@ -6,6 +6,12 @@ struct User { double fValue = 0; + User &operator=(double value) + { + fValue = value; + return *this; + } + User &operator++() { fValue++; @@ -119,3 +125,19 @@ TEST(RHistEngineUser, FillWeight) std::array indices = {9}; EXPECT_EQ(engine.GetBinContent(indices).fValue, 0.9); } + +TEST(RHistEngineUser, SetBinContent) +{ + // Setting uses operator= + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engine({axis}); + + const RBinIndex index(7); + engine.SetBinContent(index, 42); + EXPECT_EQ(engine.GetBinContent(index).fValue, 42); + + const std::array indices = {index}; + engine.SetBinContent(indices, 43); + EXPECT_EQ(engine.GetBinContent(indices).fValue, 43); +}