From ef9e91f898234e9daa9b45c080edceea15baacbe Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Thu, 4 Dec 2025 14:50:46 +0100 Subject: [PATCH 1/2] [hist] Extend testing of RHist{,Engine} constructors --- hist/histv7/test/hist_engine.cxx | 37 ++++++++++++++++++++++---------- hist/histv7/test/hist_hist.cxx | 14 +++++++++++- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/hist/histv7/test/hist_engine.cxx b/hist/histv7/test/hist_engine.cxx index 3390e2d7ccb34..1bb7ceb0cb69c 100644 --- a/hist/histv7/test/hist_engine.cxx +++ b/hist/histv7/test/hist_engine.cxx @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -23,20 +24,25 @@ TEST(RHistEngine, Constructor) const std::vector categories = {"a", "b", "c"}; const RCategoricalAxis categoricalAxis(categories); - RHistEngine engine({regularAxis, variableBinAxis, categoricalAxis}); + // The most generic constructor takes a vector of axis objects. + const std::vector axes = {regularAxis, variableBinAxis, categoricalAxis}; + RHistEngine engine(axes); EXPECT_EQ(engine.GetNDimensions(), 3); - const auto &axes = engine.GetAxes(); - ASSERT_EQ(axes.size(), 3); - EXPECT_EQ(axes[0].index(), 0); - EXPECT_EQ(axes[1].index(), 1); - EXPECT_EQ(axes[2].index(), 2); - EXPECT_TRUE(std::get_if(&axes[0]) != nullptr); - EXPECT_TRUE(std::get_if(&axes[1]) != nullptr); - EXPECT_TRUE(std::get_if(&axes[2]) != nullptr); - - // Both axes include underflow and overflow bins. + { + const auto &engineAxes = engine.GetAxes(); + ASSERT_EQ(engineAxes.size(), 3); + EXPECT_EQ(engineAxes[0].index(), 0); + EXPECT_EQ(engineAxes[1].index(), 1); + EXPECT_EQ(engineAxes[2].index(), 2); + EXPECT_TRUE(std::get_if(&engineAxes[0]) != nullptr); + EXPECT_TRUE(std::get_if(&engineAxes[1]) != nullptr); + EXPECT_TRUE(std::get_if(&engineAxes[2]) != nullptr); + } + + // All axes include underflow and overflow bins. EXPECT_EQ(engine.GetTotalNBins(), (BinsX + 2) * (BinsY + 2) * (categories.size() + 1)); + // Test other constructors, including move-assignment. engine = RHistEngine(BinsX, {0, BinsX}); ASSERT_EQ(engine.GetNDimensions(), 1); auto *regular = std::get_if(&engine.GetAxes()[0]); @@ -44,6 +50,15 @@ TEST(RHistEngine, Constructor) EXPECT_EQ(regular->GetNNormalBins(), BinsX); EXPECT_EQ(regular->GetLow(), 0); EXPECT_EQ(regular->GetHigh(), BinsX); + // std::make_pair will take the types of the arguments, std::size_t in this case. + engine = RHistEngine(BinsX, std::make_pair(0, BinsX)); + EXPECT_EQ(engine.GetNDimensions(), 1); + + // Brace-enclosed initializer list + engine = RHistEngine({variableBinAxis}); + EXPECT_EQ(engine.GetNDimensions(), 1); + engine = RHistEngine({variableBinAxis, categoricalAxis}); + EXPECT_EQ(engine.GetNDimensions(), 2); } TEST(RHistEngine, GetBinContentInvalidNumberOfArguments) diff --git a/hist/histv7/test/hist_hist.cxx b/hist/histv7/test/hist_hist.cxx index 8bcbc12d48b24..5c0fb1e025dd6 100644 --- a/hist/histv7/test/hist_hist.cxx +++ b/hist/histv7/test/hist_hist.cxx @@ -16,7 +16,9 @@ TEST(RHist, Constructor) static constexpr std::size_t Bins = 20; const RRegularAxis regularAxis(Bins, {0, Bins}); - RHist hist({regularAxis, regularAxis}); + // The most generic constructor takes a vector of axis objects. + const std::vector axes = {regularAxis, regularAxis}; + RHist hist(axes); EXPECT_EQ(hist.GetNDimensions(), 2); const auto &engine = hist.GetEngine(); EXPECT_EQ(engine.GetNDimensions(), 2); @@ -26,6 +28,7 @@ TEST(RHist, Constructor) // Both axes include underflow and overflow bins. EXPECT_EQ(hist.GetTotalNBins(), (Bins + 2) * (Bins + 2)); + // Test other constructors, including move-assignment. hist = RHist(Bins, {0, Bins}); ASSERT_EQ(hist.GetNDimensions(), 1); auto *regular = std::get_if(&hist.GetAxes()[0]); @@ -33,6 +36,15 @@ TEST(RHist, Constructor) EXPECT_EQ(regular->GetNNormalBins(), Bins); EXPECT_EQ(regular->GetLow(), 0); EXPECT_EQ(regular->GetHigh(), Bins); + // std::make_pair will take the types of the arguments, std::size_t in this case. + hist = RHist(Bins, std::make_pair(0, Bins)); + EXPECT_EQ(hist.GetNDimensions(), 1); + + // Brace-enclosed initializer list + hist = RHist({regularAxis}); + EXPECT_EQ(hist.GetNDimensions(), 1); + hist = RHist({regularAxis, regularAxis}); + EXPECT_EQ(hist.GetNDimensions(), 2); } TEST(RHist, Add) From 982347880bcbd5405432fbb0a80449715587f1fe Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Thu, 4 Dec 2025 13:35:09 +0100 Subject: [PATCH 2/2] [hist] More constructors for RHistEngine and RHist They simplify creation of multi-dimensional histograms in user code. --- hist/histv7/inc/ROOT/RHist.hxx | 25 +++++++++++++++++++++++-- hist/histv7/inc/ROOT/RHistEngine.hxx | 23 ++++++++++++++++++++++- hist/histv7/test/hist_engine.cxx | 8 ++++++++ hist/histv7/test/hist_hist.cxx | 8 ++++++++ 4 files changed, 61 insertions(+), 3 deletions(-) diff --git a/hist/histv7/inc/ROOT/RHist.hxx b/hist/histv7/inc/ROOT/RHist.hxx index 84de3c8a33e38..c5f1d43ccaf70 100644 --- a/hist/histv7/inc/ROOT/RHist.hxx +++ b/hist/histv7/inc/ROOT/RHist.hxx @@ -75,14 +75,35 @@ public: /// \param[in] axes the axis objects, must have size > 0 explicit RHist(std::vector axes) : fEngine(std::move(axes)), fStats(fEngine.GetNDimensions()) {} - /// Construct a one-dimensional histogram engine with a regular axis. + /// Construct a histogram. + /// + /// Note that there is no perfect forwarding of the axis objects. If that is needed, use the + /// \ref RHist(std::vector axes) "overload accepting a std::vector". + /// + /// \param[in] axes the axis objects, must have size > 0 + explicit RHist(std::initializer_list axes) : RHist(std::vector(axes)) {} + + /// Construct a histogram. + /// + /// Note that there is no perfect forwarding of the axis objects. If that is needed, use the + /// \ref RHist(std::vector axes) "overload accepting a std::vector". + /// + /// \param[in] axis1 the first axis object + /// \param[in] axes the remaining axis objects + template + explicit RHist(const RAxisVariant &axis1, const Axes &...axes) : RHist(std::vector{axis1, axes...}) + { + } + + /// Construct a one-dimensional histogram with a regular axis. /// /// \param[in] nNormalBins the number of normal bins, must be > 0 /// \param[in] interval the axis interval (lower end inclusive, upper end exclusive) /// \par See also /// the \ref RRegularAxis::RRegularAxis(std::uint64_t nNormalBins, std::pair interval, bool /// enableFlowBins) "constructor of RRegularAxis" - RHist(std::uint64_t nNormalBins, std::pair interval) : RHist({RRegularAxis(nNormalBins, interval)}) + RHist(std::uint64_t nNormalBins, std::pair interval) + : RHist(std::vector{RRegularAxis(nNormalBins, interval)}) { } diff --git a/hist/histv7/inc/ROOT/RHistEngine.hxx b/hist/histv7/inc/ROOT/RHistEngine.hxx index 02c1038dedb29..9710349d08796 100644 --- a/hist/histv7/inc/ROOT/RHistEngine.hxx +++ b/hist/histv7/inc/ROOT/RHistEngine.hxx @@ -84,6 +84,27 @@ public: fBinContents.resize(fAxes.ComputeTotalNBins()); } + /// Construct a histogram engine. + /// + /// Note that there is no perfect forwarding of the axis objects. If that is needed, use the + /// \ref RHistEngine(std::vector axes) "overload accepting a std::vector". + /// + /// \param[in] axes the axis objects, must have size > 0 + explicit RHistEngine(std::initializer_list axes) : RHistEngine(std::vector(axes)) {} + + /// Construct a histogram engine. + /// + /// Note that there is no perfect forwarding of the axis objects. If that is needed, use the + /// \ref RHistEngine(std::vector axes) "overload accepting a std::vector". + /// + /// \param[in] axis1 the first axis object + /// \param[in] axes the remaining axis objects + template + explicit RHistEngine(const RAxisVariant &axis1, const Axes &...axes) + : RHistEngine(std::vector{axis1, axes...}) + { + } + /// Construct a one-dimensional histogram engine with a regular axis. /// /// \param[in] nNormalBins the number of normal bins, must be > 0 @@ -92,7 +113,7 @@ public: /// the \ref RRegularAxis::RRegularAxis(std::uint64_t nNormalBins, std::pair interval, bool /// enableFlowBins) "constructor of RRegularAxis" RHistEngine(std::uint64_t nNormalBins, std::pair interval) - : RHistEngine({RRegularAxis(nNormalBins, interval)}) + : RHistEngine(std::vector{RRegularAxis(nNormalBins, interval)}) { } diff --git a/hist/histv7/test/hist_engine.cxx b/hist/histv7/test/hist_engine.cxx index 1bb7ceb0cb69c..db559c6d7d538 100644 --- a/hist/histv7/test/hist_engine.cxx +++ b/hist/histv7/test/hist_engine.cxx @@ -59,6 +59,14 @@ TEST(RHistEngine, Constructor) EXPECT_EQ(engine.GetNDimensions(), 1); engine = RHistEngine({variableBinAxis, categoricalAxis}); EXPECT_EQ(engine.GetNDimensions(), 2); + + // Templated constructors + engine = RHistEngine(variableBinAxis); + EXPECT_EQ(engine.GetNDimensions(), 1); + engine = RHistEngine(variableBinAxis, categoricalAxis); + EXPECT_EQ(engine.GetNDimensions(), 2); + engine = RHistEngine(variableBinAxis, categoricalAxis, regularAxis); + EXPECT_EQ(engine.GetNDimensions(), 3); } TEST(RHistEngine, GetBinContentInvalidNumberOfArguments) diff --git a/hist/histv7/test/hist_hist.cxx b/hist/histv7/test/hist_hist.cxx index 5c0fb1e025dd6..3ef56b957bf67 100644 --- a/hist/histv7/test/hist_hist.cxx +++ b/hist/histv7/test/hist_hist.cxx @@ -45,6 +45,14 @@ TEST(RHist, Constructor) EXPECT_EQ(hist.GetNDimensions(), 1); hist = RHist({regularAxis, regularAxis}); EXPECT_EQ(hist.GetNDimensions(), 2); + + // Templated constructors + hist = RHist(regularAxis); + EXPECT_EQ(hist.GetNDimensions(), 1); + hist = RHist(regularAxis, regularAxis); + EXPECT_EQ(hist.GetNDimensions(), 2); + hist = RHist(regularAxis, regularAxis, regularAxis); + EXPECT_EQ(hist.GetNDimensions(), 3); } TEST(RHist, Add)