[hist] Check arguments in axis constructors

hist/histv7/inc/ROOT/RRegularAxis.hxx

@@ -9,6 +9,7 @@
 
 #include <cstddef>
 #include <stdexcept>
+#include <string>
 
 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);
    }
 

hist/histv7/inc/ROOT/RVariableBinAxis.hxx

@@ -9,6 +9,7 @@
 
 #include <cstddef>
 #include <stdexcept>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -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<double> 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; }

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<double>::infinity();
    static constexpr double NaN = std::numeric_limits<double>::quiet_NaN();

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<double>::infinity();
    static constexpr double NaN = std::numeric_limits<double>::quiet_NaN();