diff --git a/hist/histv7/inc/ROOT/RHist.hxx b/hist/histv7/inc/ROOT/RHist.hxx index 1fdf184512438..d8e269ad6e193 100644 --- a/hist/histv7/inc/ROOT/RHist.hxx +++ b/hist/histv7/inc/ROOT/RHist.hxx @@ -32,11 +32,11 @@ hist.Fill(8.5); // hist.GetBinContent(ROOT::Experimental::RBinIndex(3)) will return 1 \endcode -The class is templated on the bin content type. For counting, as in the example above, it may be an integer type such as -`int` or `long`. Narrower types such as `unsigned char` or `short` are supported, but may overflow due to their limited -range and must be used with care. For weighted filling, the bin content type must be a floating-point type such as -`float` or `double`, or the special type RBinWithError. Note that `float` has a limited significand precision of 24 -bits. +The class is templated on the bin content type. For counting, as in the example above, it may be an integral type such +as `int` or `long`. Narrower types such as `unsigned char` or `short` are supported, but may overflow due to their +limited range and must be used with care. For weighted filling, the bin content type must not be an integral type, but +a floating-point type such as `float` or `double`, or the special type RBinWithError. Note that `float` has a limited +significand precision of 24 bits. An object can have arbitrary dimensionality determined at run-time. The axis configuration is passed as a vector of RAxisVariant: @@ -219,8 +219,7 @@ public: /// Fill an entry into the histogram with a weight. /// - /// This overload is only available for floating-point bin content types (see - /// \ref RHistEngine::SupportsWeightedFilling). + /// This overload is not available for integral bin content types (see \ref RHistEngine::SupportsWeightedFilling). /// /// \code /// ROOT::Experimental::RHist hist({/* two dimensions */}); @@ -257,7 +256,7 @@ public: /// ROOT::Experimental::RHist hist({/* two dimensions */}); /// hist.Fill(8.5, 10.5, ROOT::Experimental::RWeight(0.8)); /// \endcode - /// This is only available for floating-point bin content types (see \ref RHistEngine::SupportsWeightedFilling). + /// This is not available for integral bin content types (see \ref RHistEngine::SupportsWeightedFilling). /// /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently /// discarded. diff --git a/hist/histv7/inc/ROOT/RHistEngine.hxx b/hist/histv7/inc/ROOT/RHistEngine.hxx index d1b6b99d10b46..1584b54fc75b3 100644 --- a/hist/histv7/inc/ROOT/RHistEngine.hxx +++ b/hist/histv7/inc/ROOT/RHistEngine.hxx @@ -7,7 +7,6 @@ #include "RAxes.hxx" #include "RBinIndex.hxx" -#include "RBinWithError.hxx" #include "RHistUtils.hxx" #include "RLinearizedIndex.hxx" #include "RRegularAxis.hxx" @@ -37,11 +36,11 @@ hist.Fill(8.5); // hist.GetBinContent(ROOT::Experimental::RBinIndex(3)) will return 1 \endcode -The class is templated on the bin content type. For counting, as in the example above, it may be an integer type such as -`int` or `long`. Narrower types such as `unsigned char` or `short` are supported, but may overflow due to their limited -range and must be used with care. For weighted filling, the bin content type must be a floating-point type such as -`float` or `double`, or the special type RBinWithError. Note that `float` has a limited significand precision of 24 -bits. +The class is templated on the bin content type. For counting, as in the example above, it may be an integral type such +as `int` or `long`. Narrower types such as `unsigned char` or `short` are supported, but may overflow due to their +limited range and must be used with care. For weighted filling, the bin content type must not be an integral type, but +a floating-point type such as `float` or `double`, or the special type RBinWithError. Note that `float` has a limited +significand precision of 24 bits. An object can have arbitrary dimensionality determined at run-time. The axis configuration is passed as a vector of RAxisVariant: @@ -209,8 +208,7 @@ public: } /// Whether this histogram engine type supports weighted filling. - static constexpr bool SupportsWeightedFilling = - std::is_floating_point_v || std::is_same_v; + static constexpr bool SupportsWeightedFilling = !std::is_integral_v; /// Fill an entry into the histogram. /// @@ -246,7 +244,7 @@ public: /// Fill an entry into the histogram with a weight. /// - /// This overload is only available for floating-point bin content types (see \ref SupportsWeightedFilling). + /// This overload is not available for integral bin content types (see \ref SupportsWeightedFilling). /// /// \code /// ROOT::Experimental::RHistEngine hist({/* two dimensions */}); @@ -267,7 +265,7 @@ public: template void Fill(const std::tuple &args, RWeight weight) { - static_assert(SupportsWeightedFilling, "weighted filling is only supported for floating-point bin content types"); + static_assert(SupportsWeightedFilling, "weighted filling is not supported for integral bin content types"); // We could rely on RAxes::ComputeGlobalIndex to check the number of arguments, but its exception message might // be confusing for users. @@ -293,7 +291,7 @@ public: /// ROOT::Experimental::RHistEngine hist({/* two dimensions */}); /// hist.Fill(8.5, 10.5, ROOT::Experimental::RWeight(0.8)); /// \endcode - /// This is only available for floating-point bin content types (see \ref SupportsWeightedFilling). + /// This is not available for integral bin content types (see \ref SupportsWeightedFilling). /// /// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently /// discarded. @@ -309,8 +307,7 @@ public: { auto t = std::forward_as_tuple(args...); if constexpr (std::is_same_v::type, RWeight>) { - static_assert(SupportsWeightedFilling, - "weighted filling is only supported for floating-point bin content types"); + static_assert(SupportsWeightedFilling, "weighted filling is not supported for integral bin content types"); static constexpr std::size_t N = sizeof...(A) - 1; if (N != fAxes.GetNDimensions()) { throw std::invalid_argument("invalid number of arguments to Fill"); diff --git a/hist/histv7/test/CMakeLists.txt b/hist/histv7/test/CMakeLists.txt index 52e297af87fb0..e8f210118b41e 100644 --- a/hist/histv7/test/CMakeLists.txt +++ b/hist/histv7/test/CMakeLists.txt @@ -4,6 +4,7 @@ HIST_ADD_GTEST(hist_hist hist_hist.cxx) HIST_ADD_GTEST(hist_index hist_index.cxx) HIST_ADD_GTEST(hist_regular hist_regular.cxx) HIST_ADD_GTEST(hist_stats hist_stats.cxx) +HIST_ADD_GTEST(hist_user hist_user.cxx) HIST_ADD_GTEST(hist_variable hist_variable.cxx) if(NOT DEFINED histv7_standalone) diff --git a/hist/histv7/test/hist_hist.cxx b/hist/histv7/test/hist_hist.cxx index 34aceaa451ee5..a163a3e1ae904 100644 --- a/hist/histv7/test/hist_hist.cxx +++ b/hist/histv7/test/hist_hist.cxx @@ -120,6 +120,10 @@ TEST(RHist, FillWeight) hist.Fill(8.5, RWeight(0.8)); hist.Fill(std::make_tuple(9.5), RWeight(0.9)); + EXPECT_FLOAT_EQ(hist.GetBinContent(RBinIndex(8)), 0.8); + std::array indices = {9}; + EXPECT_FLOAT_EQ(hist.GetBinContent(indices), 0.9); + EXPECT_EQ(hist.GetStats().GetNEntries(), 2); // Cross-checked with TH1 EXPECT_FLOAT_EQ(hist.GetStats().ComputeNEffectiveEntries(), 1.9931034); diff --git a/hist/histv7/test/hist_user.cxx b/hist/histv7/test/hist_user.cxx new file mode 100644 index 0000000000000..cb893b46520ab --- /dev/null +++ b/hist/histv7/test/hist_user.cxx @@ -0,0 +1,121 @@ +#include "hist_test.hxx" + +#include + +// User-defined bin content type consisting of a single double, but can be much more complicated. +struct User { + double fValue = 0; + + User &operator++() + { + fValue++; + return *this; + } + + User operator++(int) + { + User old = *this; + operator++(); + return old; + } + + User &operator+=(double w) + { + fValue += w; + return *this; + } + + User &operator+=(const User &rhs) + { + fValue += rhs.fValue; + return *this; + } +}; + +static_assert(std::is_nothrow_move_constructible_v>); +static_assert(std::is_nothrow_move_assignable_v>); + +static_assert(std::is_nothrow_move_constructible_v>); +static_assert(std::is_nothrow_move_assignable_v>); + +TEST(RHistEngineUser, Add) +{ + // Addition uses operator+=(const User &) + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engineA({axis}); + RHistEngine engineB({axis}); + + engineA.Fill(8.5); + engineB.Fill(9.5); + + engineA.Add(engineB); + + EXPECT_EQ(engineA.GetBinContent(RBinIndex(8)).fValue, 1); + EXPECT_EQ(engineA.GetBinContent(RBinIndex(9)).fValue, 1); +} + +TEST(RHistEngineUser, Clear) +{ + // Clearing assigns default-constructed objects. + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engine({axis}); + + engine.Fill(8.5); + engine.Fill(9.5); + + engine.Clear(); + + EXPECT_EQ(engine.GetBinContent(RBinIndex(8)).fValue, 0); + EXPECT_EQ(engine.GetBinContent(RBinIndex(9)).fValue, 0); +} + +TEST(RHistEngineUser, Clone) +{ + // Cloning copy-assigns the objects. + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engineA({axis}); + + engineA.Fill(8.5); + + RHistEngine engineB = engineA.Clone(); + EXPECT_EQ(engineB.GetBinContent(8).fValue, 1); + + // Check that we can continue filling the clone. + engineB.Fill(9.5); + + EXPECT_EQ(engineA.GetBinContent(9).fValue, 0); + EXPECT_EQ(engineB.GetBinContent(9).fValue, 1); +} + +TEST(RHistEngineUser, Fill) +{ + // Unweighted filling uses operator++(int) + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engine({axis}); + + engine.Fill(8.5); + engine.Fill(std::make_tuple(9.5)); + + EXPECT_EQ(engine.GetBinContent(RBinIndex(8)).fValue, 1); + std::array indices = {9}; + EXPECT_EQ(engine.GetBinContent(indices).fValue, 1); +} + +TEST(RHistEngineUser, FillWeight) +{ + // Weighted filling uses operator+=(double) + static constexpr std::size_t Bins = 20; + const RRegularAxis axis(Bins, {0, Bins}); + RHistEngine engine({axis}); + + engine.Fill(8.5, RWeight(0.8)); + engine.Fill(std::make_tuple(9.5), RWeight(0.9)); + + EXPECT_EQ(engine.GetBinContent(RBinIndex(8)).fValue, 0.8); + std::array indices = {9}; + EXPECT_EQ(engine.GetBinContent(indices).fValue, 0.9); +}