Skip to content

Commit 48092ca

Browse files
committed
[hist] Check argument types during ComputeGlobalIndex
This prepares for future axis types that accept different argument types (in particular RCategoricalAxis), where we can only decide at run-time if the parameters match the axis configuration. However, because the type check is evaluated at compile-time, there is no performance penalty for cases where the type is convertible.
1 parent 943ee82 commit 48092ca

File tree

6 files changed

+55
-10
lines changed

6 files changed

+55
-10
lines changed

hist/histv7/inc/ROOT/RAxes.hxx

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <cstddef>
1616
#include <stdexcept>
1717
#include <tuple>
18+
#include <type_traits>
1819
#include <utility>
1920
#include <variant>
2021
#include <vector>
@@ -81,14 +82,23 @@ private:
8182
template <std::size_t I, std::size_t N, typename... A>
8283
RLinearizedIndex ComputeGlobalIndexImpl(std::size_t index, const std::tuple<A...> &args) const
8384
{
85+
using ArgumentType = std::tuple_element_t<I, std::tuple<A...>>;
8486
const auto &axis = fAxes[I];
8587
RLinearizedIndex linIndex;
8688
if (auto *regular = std::get_if<RRegularAxis>(&axis)) {
87-
index *= regular->GetTotalNBins();
88-
linIndex = regular->ComputeLinearizedIndex(std::get<I>(args));
89+
if constexpr (std::is_convertible_v<ArgumentType, RRegularAxis::ArgumentType>) {
90+
index *= regular->GetTotalNBins();
91+
linIndex = regular->ComputeLinearizedIndex(std::get<I>(args));
92+
} else {
93+
throw std::invalid_argument("invalid type of argument");
94+
}
8995
} else if (auto *variable = std::get_if<RVariableBinAxis>(&axis)) {
90-
index *= variable->GetTotalNBins();
91-
linIndex = variable->ComputeLinearizedIndex(std::get<I>(args));
96+
if constexpr (std::is_convertible_v<ArgumentType, RVariableBinAxis::ArgumentType>) {
97+
index *= variable->GetTotalNBins();
98+
linIndex = variable->ComputeLinearizedIndex(std::get<I>(args));
99+
} else {
100+
throw std::invalid_argument("invalid type of argument");
101+
}
92102
} else {
93103
throw std::logic_error("unimplemented axis type"); // GCOVR_EXCL_LINE
94104
}
@@ -111,6 +121,9 @@ private:
111121
public:
112122
/// Compute the global index for all axes.
113123
///
124+
/// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be
125+
/// converted for the axis type at run-time.
126+
///
114127
/// \param[in] args the arguments
115128
/// \return the global index that may be invalid
116129
template <typename... A>

hist/histv7/inc/ROOT/RHist.hxx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@ public:
204204
/// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently
205205
/// discarded.
206206
///
207-
/// Throws an exception if the number of arguments does not match the axis configuration.
207+
/// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be
208+
/// converted for the axis type at run-time.
208209
///
209210
/// \param[in] args the arguments for each axis
210211
/// \par See also
@@ -231,7 +232,8 @@ public:
231232
/// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently
232233
/// discarded.
233234
///
234-
/// Throws an exception if the number of arguments does not match the axis configuration.
235+
/// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be
236+
/// converted for the axis type at run-time.
235237
///
236238
/// \param[in] args the arguments for each axis
237239
/// \param[in] weight the weight for this entry
@@ -262,7 +264,8 @@ public:
262264
/// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently
263265
/// discarded.
264266
///
265-
/// Throws an exception if the number of arguments does not match the axis configuration.
267+
/// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be
268+
/// converted for the axis type at run-time.
266269
///
267270
/// \param[in] args the arguments for each axis
268271
/// \par See also

hist/histv7/inc/ROOT/RHistEngine.hxx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,8 @@ public:
223223
/// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently
224224
/// discarded.
225225
///
226-
/// Throws an exception if the number of arguments does not match the axis configuration.
226+
/// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be
227+
/// converted for the axis type at run-time.
227228
///
228229
/// \param[in] args the arguments for each axis
229230
/// \par See also
@@ -257,7 +258,8 @@ public:
257258
/// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently
258259
/// discarded.
259260
///
260-
/// Throws an exception if the number of arguments does not match the axis configuration.
261+
/// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be
262+
/// converted for the axis type at run-time.
261263
///
262264
/// \param[in] args the arguments for each axis
263265
/// \param[in] weight the weight for this entry
@@ -298,7 +300,8 @@ public:
298300
/// If one of the arguments is outside the corresponding axis and flow bins are disabled, the entry will be silently
299301
/// discarded.
300302
///
301-
/// Throws an exception if the number of arguments does not match the axis configuration.
303+
/// Throws an exception if the number of arguments does not match the axis configuration, or if an argument cannot be
304+
/// converted for the axis type at run-time.
302305
///
303306
/// \param[in] args the arguments for each axis
304307
/// \par See also

hist/histv7/inc/ROOT/RRegularAxis.hxx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ outside the axis will be silently discarded.
3535
Feedback is welcome!
3636
*/
3737
class RRegularAxis final {
38+
public:
39+
using ArgumentType = double;
40+
41+
private:
3842
/// The number of normal bins
3943
std::size_t fNNormalBins;
4044
/// The lower end of the axis interval

hist/histv7/inc/ROOT/RVariableBinAxis.hxx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ outside the axis will be silently discarded.
3737
Feedback is welcome!
3838
*/
3939
class RVariableBinAxis final {
40+
public:
41+
using ArgumentType = double;
42+
43+
private:
4044
/// The (ordered) edges of the normal bins
4145
std::vector<double> fBinEdges;
4246
/// Whether underflow and overflow bins are enabled

hist/histv7/test/hist_axes.cxx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,3 +203,21 @@ TEST(RAxes, ComputeGlobalIndexInvalidNumberOfArguments)
203203
EXPECT_NO_THROW(axes2.ComputeGlobalIndex(indices2));
204204
EXPECT_THROW(axes2.ComputeGlobalIndex(indices3), std::invalid_argument);
205205
}
206+
207+
TEST(RAxes, ComputeGlobalIndexInvalidArgumentType)
208+
{
209+
static constexpr std::size_t BinsX = 20;
210+
const RRegularAxis regularAxis(BinsX, {0, BinsX});
211+
static constexpr std::size_t BinsY = 30;
212+
std::vector<double> bins;
213+
for (std::size_t i = 0; i < BinsY; i++) {
214+
bins.push_back(i);
215+
}
216+
bins.push_back(BinsY);
217+
const RVariableBinAxis variableBinAxis(bins);
218+
const RAxes axes({regularAxis, variableBinAxis});
219+
220+
EXPECT_NO_THROW(axes.ComputeGlobalIndex(std::make_tuple(1, 2)));
221+
EXPECT_THROW(axes.ComputeGlobalIndex(std::make_tuple("1", 2)), std::invalid_argument);
222+
EXPECT_THROW(axes.ComputeGlobalIndex(std::make_tuple(1, "2")), std::invalid_argument);
223+
}

0 commit comments

Comments
 (0)