Skip to content

Commit 096d573

Browse files
committed
[hist] Test against long long truncation
Add and use function Internal::SetBinContent not exposed in the public interface.
1 parent 98bde47 commit 096d573

File tree

4 files changed

+59
-2
lines changed

4 files changed

+59
-2
lines changed

hist/histv7/inc/ROOT/RHistEngine.hxx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ class TBuffer;
2525
namespace ROOT {
2626
namespace Experimental {
2727

28+
// forward declarations for friend declaration
29+
template <typename BinContentType>
30+
class RHistEngine;
31+
namespace Internal {
32+
template <typename T, std::size_t N>
33+
static void
34+
SetBinContent(RHistEngine<T> &hist, const std::array<RBinIndex, N> &indices, const T &value);
35+
} // namespace Internal
36+
2837
/**
2938
A histogram data structure to bin data along multiple dimensions.
3039
@@ -57,6 +66,9 @@ Feedback is welcome!
5766
*/
5867
template <typename BinContentType>
5968
class RHistEngine final {
69+
template <typename T, std::size_t N>
70+
friend void Internal::SetBinContent(RHistEngine<T> &, const std::array<RBinIndex, N> &, const T &);
71+
6072
/// The axis configuration for this histogram. Relevant methods are forwarded from the public interface.
6173
Internal::RAxes fAxes;
6274
/// The bin contents for this histogram
@@ -330,6 +342,21 @@ public:
330342
void Streamer(TBuffer &) { throw std::runtime_error("unable to store RHistEngine"); }
331343
};
332344

345+
namespace Internal {
346+
/// %Internal function to set the content of a single bin.
347+
template <typename T, std::size_t N>
348+
static void
349+
SetBinContent(RHistEngine<T> &hist, const std::array<RBinIndex, N> &indices, const T &value)
350+
{
351+
RLinearizedIndex index = hist.fAxes.ComputeGlobalIndex(indices);
352+
if (!index.fValid) {
353+
throw std::invalid_argument("bin not found in SetBinContent");
354+
}
355+
assert(index.fIndex < hist.fBinContents.size());
356+
hist.fBinContents[index.fIndex] = value;
357+
}
358+
} // namespace Internal
359+
333360
} // namespace Experimental
334361
} // namespace ROOT
335362

hist/histv7/test/hist_engine.cxx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "hist_test.hxx"
22

3+
#include <array>
34
#include <stdexcept>
45
#include <type_traits>
56
#include <variant>
@@ -71,6 +72,23 @@ TEST(RHistEngine, GetBinContentNotFound)
7172
EXPECT_THROW(engine.GetBinContent(Bins), std::invalid_argument);
7273
}
7374

75+
TEST(RHistEngine, SetBinContent)
76+
{
77+
using ROOT::Experimental::Internal::SetBinContent;
78+
79+
static constexpr std::size_t Bins = 20;
80+
const RRegularAxis axis(Bins, {0, Bins});
81+
RHistEngine<int> engine({axis});
82+
83+
std::array<RBinIndex, 1> indices = {7};
84+
SetBinContent(engine, indices, 42);
85+
EXPECT_EQ(engine.GetBinContent(indices), 42);
86+
87+
// "bin not found"
88+
indices = {Bins};
89+
EXPECT_THROW(SetBinContent(engine, indices, 43), std::invalid_argument);
90+
}
91+
7492
TEST(RHistEngine, Add)
7593
{
7694
static constexpr std::size_t Bins = 20;

hist/histv7util/test/hist_convert_TH1.cxx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <TAxis.h>
44
#include <TH1.h>
55

6+
#include <array>
67
#include <cstddef>
78
#include <memory>
89
#include <stdexcept>
@@ -142,11 +143,21 @@ TEST(ConvertToTH1L, RHistEngine)
142143
auto th1l = ConvertToTH1L(engineL);
143144
ASSERT_TRUE(th1l);
144145

145-
const RHistEngine<long long> engineLL(Bins, {0, Bins});
146+
RHistEngine<long long> engineLL(Bins, {0, Bins});
147+
148+
// Set one 64-bit long long value larger than what double can exactly represent.
149+
static constexpr long long Large = (1LL << 60) - 1;
150+
const std::array<RBinIndex, 1> indices = {1};
151+
ROOT::Experimental::Internal::SetBinContent(engineLL, indices, Large);
152+
146153
th1l = ConvertToTH1L(engineLL);
147154
ASSERT_TRUE(th1l);
148155

149-
// TODO: Test that 64-bit long long values are not truncated to double
156+
// Get the value via TArrayL::At and store into a variable to be sure about the type. During direct comparison, a
157+
// double return value may automatically promate Large to a double as well, introducing the truncation we want to
158+
// test against.
159+
const long long value = th1l->At(2);
160+
EXPECT_EQ(value, Large);
150161
}
151162

152163
TEST(ConvertToTH1L, RHist)

hist/histv7util/test/histutil_test.hxx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <ROOT/RWeight.hxx>
99

1010
using namespace ROOT::Experimental::Hist;
11+
using ROOT::Experimental::RBinIndex;
1112
using ROOT::Experimental::RBinWithError;
1213
using ROOT::Experimental::RHist;
1314
using ROOT::Experimental::RHistEngine;

0 commit comments

Comments
 (0)