diff --git a/hist/histv7/inc/ROOT/RRegularAxis.hxx b/hist/histv7/inc/ROOT/RRegularAxis.hxx index aa68d641c32aa..a31984f67871d 100644 --- a/hist/histv7/inc/ROOT/RRegularAxis.hxx +++ b/hist/histv7/inc/ROOT/RRegularAxis.hxx @@ -9,6 +9,7 @@ #include #include +#include class TBuffer; @@ -44,14 +45,20 @@ class RRegularAxis final { public: /// Construct a regular axis object. /// - /// \param[in] numNormalBins the number of normal bins + /// \param[in] numNormalBins the number of normal bins, must be > 0 /// \param[in] low the lower end of the axis interval (inclusive) - /// \param[in] high the upper end of the axis interval (exclusive) + /// \param[in] high the upper end of the axis interval (exclusive), must be > low /// \param[in] enableFlowBins whether to enable underflow and overflow bins RRegularAxis(std::size_t numNormalBins, double low, double high, bool enableFlowBins = true) : fNumNormalBins(numNormalBins), fLow(low), fHigh(high), fEnableFlowBins(enableFlowBins) { - // FIXME: should validate numNormalBins > 0 and low < high + if (numNormalBins == 0) { + throw std::invalid_argument("numNormalBins must be > 0"); + } + if (low >= high) { + std::string msg = "high must be > low, but " + std::to_string(low) + " >= " + std::to_string(high); + throw std::invalid_argument(msg); + } fInvBinWidth = numNormalBins / (high - low); } diff --git a/hist/histv7/inc/ROOT/RVariableBinAxis.hxx b/hist/histv7/inc/ROOT/RVariableBinAxis.hxx index fbe5f3c94583a..9a4eabc981a95 100644 --- a/hist/histv7/inc/ROOT/RVariableBinAxis.hxx +++ b/hist/histv7/inc/ROOT/RVariableBinAxis.hxx @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -41,12 +42,21 @@ class RVariableBinAxis final { public: /// Construct an axis object with variable bins. /// - /// \param[in] binEdges the (ordered) edges of the normal bins + /// \param[in] binEdges the (ordered) edges of the normal bins, must define at least one bin (i.e. size >= 2) /// \param[in] enableFlowBins whether to enable underflow and overflow bins RVariableBinAxis(std::vector binEdges, bool enableFlowBins = true) : fBinEdges(std::move(binEdges)), fEnableFlowBins(enableFlowBins) { - // FIXME: should validate that fBinEdges is sorted + if (fBinEdges.size() < 2) { + throw std::invalid_argument("must have >= 2 bin edges"); + } + for (std::size_t i = 1; i < fBinEdges.size(); i++) { + if (fBinEdges[i - 1] >= fBinEdges[i]) { + std::string msg = "binEdges must be sorted, but for bin " + std::to_string(i - 1) + ": "; + msg += std::to_string(fBinEdges[i - 1]) + " >= " + std::to_string(fBinEdges[i]); + throw std::invalid_argument(msg); + } + } } std::size_t GetNumNormalBins() const { return fBinEdges.size() - 1; } diff --git a/hist/histv7/test/hist_regular.cxx b/hist/histv7/test/hist_regular.cxx index 7c28c1816653f..e3ce3f5fea2f4 100644 --- a/hist/histv7/test/hist_regular.cxx +++ b/hist/histv7/test/hist_regular.cxx @@ -16,6 +16,9 @@ TEST(RRegularAxis, Constructor) EXPECT_EQ(axis.GetNumNormalBins(), Bins); EXPECT_EQ(axis.GetTotalNumBins(), Bins); EXPECT_FALSE(axis.HasFlowBins()); + + EXPECT_THROW(RRegularAxis(0, 0, Bins), std::invalid_argument); + EXPECT_THROW(RRegularAxis(Bins, 1, 1), std::invalid_argument); } TEST(RRegularAxis, Equality) @@ -64,6 +67,16 @@ TEST(RRegularAxis, ComputeLinearizedIndex) EXPECT_FALSE(linIndex.fValid); } + // Exactly the lower end of the axis interval + { + auto linIndex = axis.ComputeLinearizedIndex(0); + EXPECT_EQ(linIndex.fIndex, 0); + EXPECT_TRUE(linIndex.fValid); + linIndex = axisNoFlowBins.ComputeLinearizedIndex(0); + EXPECT_EQ(linIndex.fIndex, 0); + EXPECT_TRUE(linIndex.fValid); + } + for (std::size_t i = 0; i < Bins; i++) { auto linIndex = axis.ComputeLinearizedIndex(i + 0.5); EXPECT_EQ(linIndex.fIndex, i); @@ -73,6 +86,16 @@ TEST(RRegularAxis, ComputeLinearizedIndex) EXPECT_TRUE(linIndex.fValid); } + // Exactly the upper end of the axis interval + { + auto linIndex = axis.ComputeLinearizedIndex(Bins); + EXPECT_EQ(linIndex.fIndex, Bins + 1); + EXPECT_TRUE(linIndex.fValid); + linIndex = axisNoFlowBins.ComputeLinearizedIndex(Bins); + EXPECT_EQ(linIndex.fIndex, Bins + 1); + EXPECT_FALSE(linIndex.fValid); + } + // Overflow static constexpr double PositiveInfinity = std::numeric_limits::infinity(); static constexpr double NaN = std::numeric_limits::quiet_NaN(); diff --git a/hist/histv7/test/hist_variable.cxx b/hist/histv7/test/hist_variable.cxx index 1b4782c2249f8..1763710591a4a 100644 --- a/hist/histv7/test/hist_variable.cxx +++ b/hist/histv7/test/hist_variable.cxx @@ -21,6 +21,12 @@ TEST(RVariableBinAxis, Constructor) EXPECT_EQ(axis.GetNumNormalBins(), Bins); EXPECT_EQ(axis.GetTotalNumBins(), Bins); EXPECT_FALSE(axis.HasFlowBins()); + + EXPECT_THROW(RVariableBinAxis({}), std::invalid_argument); + EXPECT_THROW(RVariableBinAxis({0}), std::invalid_argument); + EXPECT_THROW(RVariableBinAxis({0, 0}), std::invalid_argument); + EXPECT_THROW(RVariableBinAxis({0, 1, 0}), std::invalid_argument); + EXPECT_THROW(RVariableBinAxis({0, 1, 1}), std::invalid_argument); } TEST(RVariableBinAxis, Equality) @@ -95,6 +101,26 @@ TEST(RVariableBinAxis, ComputeLinearizedIndex) EXPECT_TRUE(linIndex.fValid); } + // Exactly on the bin edges + for (std::size_t i = 0; i < Bins; i++) { + auto linIndex = axis.ComputeLinearizedIndex(i); + EXPECT_EQ(linIndex.fIndex, i); + EXPECT_TRUE(linIndex.fValid); + linIndex = axisNoFlowBins.ComputeLinearizedIndex(i); + EXPECT_EQ(linIndex.fIndex, i); + EXPECT_TRUE(linIndex.fValid); + } + + // Exactly the upper end of the axis interval + { + auto linIndex = axis.ComputeLinearizedIndex(Bins); + EXPECT_EQ(linIndex.fIndex, Bins + 1); + EXPECT_TRUE(linIndex.fValid); + linIndex = axisNoFlowBins.ComputeLinearizedIndex(Bins); + EXPECT_EQ(linIndex.fIndex, Bins + 1); + EXPECT_FALSE(linIndex.fValid); + } + // Overflow static constexpr double PositiveInfinity = std::numeric_limits::infinity(); static constexpr double NaN = std::numeric_limits::quiet_NaN();