From de5ddecbbda13cda1c2dd6388672069e0b1c28f2 Mon Sep 17 00:00:00 2001 From: Hans Dembinski Date: Mon, 6 Nov 2017 14:22:37 +0100 Subject: [PATCH] make iteration and bin access consistent in python and c++, exclude under/overflow bins now --- examples/example_1d.cpp | 4 +- include/boost/histogram/axis.hpp | 111 ++++++++++-------- include/boost/histogram/detail/meta.hpp | 4 +- .../histogram/histogram_impl_dynamic.hpp | 11 +- src/python/axis.cpp | 27 ++++- test/axis_test.cpp | 9 +- test/python_suite_test.py | 93 ++++++++------- 7 files changed, 153 insertions(+), 106 deletions(-) diff --git a/examples/example_1d.cpp b/examples/example_1d.cpp index b2bdfc66..0ae2f921 100644 --- a/examples/example_1d.cpp +++ b/examples/example_1d.cpp @@ -27,7 +27,7 @@ int main(int, char**) { h.fill(20.0); // put in overflow bin h.fill(0.1, bh::weight(5)); // fill with a weighted entry, weight is 5 - // iterate over bins, loop includes under- and overflow bin + // iterate over bins, loop excludes under- and overflow bins for (const auto& bin : h.axis(0_c)) { std::cout << "bin " << bin.first << " x in [" << bin.second.lower() << ", " << bin.second.upper() << "): " @@ -37,7 +37,6 @@ int main(int, char**) { /* program output: - bin -1 x in [-inf, -1): 1 +/- 1 bin 0 x in [-1, -0.7): 1 +/- 1 bin 1 x in [-0.7, -0.4): 1 +/- 1 bin 2 x in [-0.4, -0.1): 0 +/- 0 @@ -48,7 +47,6 @@ int main(int, char**) { bin 7 x in [1.1, 1.4): 1 +/- 1 bin 8 x in [1.4, 1.7): 0 +/- 0 bin 9 x in [1.7, 2): 1 +/- 1 - bin 10 x in [2, inf): 2 +/- 1.41421 */ } diff --git a/include/boost/histogram/axis.hpp b/include/boost/histogram/axis.hpp index 59b2e625..40200452 100644 --- a/include/boost/histogram/axis.hpp +++ b/include/boost/histogram/axis.hpp @@ -43,7 +43,16 @@ namespace detail { template class cref { public: cref() = default; + cref(const cref &o) : ptr_(o.ptr_) {} + cref &operator=(const cref &o) { + ptr_ = o.ptr_; + return *this; + } cref(const T &t) : ptr_(&t) {} + cref &operator=(const T &t) { + ptr_ = &t; + return *this; + } operator const T &() const { return *ptr_; } private: @@ -51,7 +60,7 @@ private: }; template -using axis_iterator_value = std::pair< +using axis_iterator_value_t = std::pair< int, typename conditional< is_reference::value, cref::type>, @@ -59,16 +68,20 @@ using axis_iterator_value = std::pair< } // namespace detail template -class axis_iterator : public iterator_facade, - detail::axis_iterator_value, - random_access_traversal_tag> { +class axis_iterator + : public iterator_facade, + detail::axis_iterator_value_t, + random_access_traversal_tag> { public: - using value_type = detail::axis_iterator_value; + using value_type = detail::axis_iterator_value_t; explicit axis_iterator(const Axis &axis, int idx) : axis_(axis) { value_.first = idx; } + axis_iterator(const axis_iterator &o) = default; + axis_iterator &operator=(const axis_iterator &o) = default; + private: void increment() noexcept { ++value_.first; } void decrement() noexcept { --value_.first; } @@ -180,10 +193,10 @@ private: namespace transform { namespace detail { struct stateless { - bool operator==(const stateless&) const noexcept { return true; } + bool operator==(const stateless &) const noexcept { return true; } template void serialize(Archive &, unsigned) {} }; -} +} // namespace detail struct identity : public detail::stateless { template static T forward(T v) { return v; } @@ -220,7 +233,7 @@ struct pow { }; } // namespace transform -/** Axis for binning real-valued data into equidistant bins. +/** Axis for equidistant intervals on the real line. * * The most common binning strategy. * Very fast. Binning is a O(1) operation. @@ -232,23 +245,24 @@ public: using bin_type = interval; using const_iterator = axis_iterator; - /** Construct axis with n bins over range [min, max). + /** Construct axis with n bins over real range [lower, upper). * * \param n number of bins. - * \param min low edge of first bin. - * \param max high edge of last bin. + * \param lower low edge of first bin. + * \param upper high edge of last bin. * \param label description of the axis. * \param uoflow whether to add under-/overflow bins. * \param trans arguments passed to the transform. */ - regular(unsigned n, value_type min, value_type max, string_view label = {}, + regular(unsigned n, value_type lower, value_type upper, + string_view label = {}, enum uoflow uo = ::boost::histogram::axis::uoflow::on, Transform trans = Transform()) : axis_base_uoflow(n, label, uo), Transform(trans), - min_(trans.forward(min)), - delta_((trans.forward(max) - trans.forward(min)) / n) { - if (!(min < max)) { - throw std::logic_error("min < max required"); + min_(trans.forward(lower)), + delta_((trans.forward(upper) - trans.forward(lower)) / n) { + if (!(lower < upper)) { + throw std::logic_error("lower < upper required"); } BOOST_ASSERT(!std::isnan(min_)); BOOST_ASSERT(!std::isnan(delta_)); @@ -286,13 +300,9 @@ public: min_ == o.min_ && delta_ == o.delta_; } - const_iterator begin() const noexcept { - return const_iterator(*this, uoflow() ? -1 : 0); - } + const_iterator begin() const { return const_iterator(*this, 0); } - const_iterator end() const noexcept { - return const_iterator(*this, uoflow() ? size() + 1 : size()); - } + const_iterator end() const { return const_iterator(*this, size()); } const Transform &transform() const noexcept { return static_cast(*this); @@ -305,7 +315,7 @@ private: template void serialize(Archive &, unsigned); }; -/** Axis for real-valued angles. +/** Axis for real values on a circle. * * The axis is circular and wraps around reaching the * perimeter value. Therefore, there are no overflow/underflow @@ -370,10 +380,10 @@ private: template void serialize(Archive &, unsigned); }; -/** An axis for real-valued data and bins of varying width. +/** Axis for non-equidistant bins on the real line. * - * Binning is a O(log(N)) operation. If speed matters - * and the problem domain allows it, prefer a regular. + * Binning is a O(log(N)) operation. If speed matters and the problem + * domain allows it, prefer a regular axis, possibly with a transform. */ template class variable : public axis_base_uoflow { public: @@ -449,13 +459,9 @@ public: return std::equal(x_.get(), x_.get() + size() + 1, o.x_.get()); } - const_iterator begin() const { - return const_iterator(*this, uoflow() ? -1 : 0); - } + const_iterator begin() const { return const_iterator(*this, 0); } - const_iterator end() const { - return const_iterator(*this, uoflow() ? size() + 1 : size()); - } + const_iterator end() const { return const_iterator(*this, size()); } private: std::unique_ptr x_; // smaller size compared to std::vector @@ -464,7 +470,7 @@ private: template void serialize(Archive &, unsigned); }; -/** An axis for a contiguous range of integers. +/** Axis for an interval of integral values with unit steps. * * Binning is a O(1) operation. This axis operates * faster than a regular. @@ -475,16 +481,18 @@ public: using bin_type = interval; using const_iterator = axis_iterator; - /** Construct axis over integer range [min, max]. + /** Construct axis over a semi-open integer interval [lower, upper). * - * \param min smallest integer of the covered range. - * \param max largest integer of the covered range. + * \param lower smallest integer of the covered range. + * \param upper largest integer of the covered range. + * \param label description of the axis. + * \param uoflow whether to add under-/overflow bins. */ - integer(value_type min, value_type max, string_view label = {}, + integer(value_type lower, value_type upper, string_view label = {}, enum uoflow uo = ::boost::histogram::axis::uoflow::on) - : axis_base_uoflow(max - min, label, uo), min_(min) { - if (min > max) { - throw std::logic_error("min <= max required"); + : axis_base_uoflow(upper - lower, label, uo), min_(lower) { + if (lower > upper) { + throw std::logic_error("lower <= upper required"); } } @@ -501,19 +509,26 @@ public: } /// Returns the integer that is mapped to the bin index. - bin_type operator[](int idx) const { return {min_ + idx, min_ + idx + 1}; } + bin_type operator[](int idx) const { + auto eval = [this](int i) { + if (i < 0) { + return -std::numeric_limits::max(); + } + if (i > size()) { + return std::numeric_limits::max(); + } + return min_ + i; + }; + return {eval(idx), eval(idx + 1)}; + } bool operator==(const integer &o) const noexcept { return axis_base_uoflow::operator==(o) && min_ == o.min_; } - const_iterator begin() const { - return const_iterator(*this, uoflow() ? -1 : 0); - } + const_iterator begin() const { return const_iterator(*this, 0); } - const_iterator end() const { - return const_iterator(*this, uoflow() ? size() + 1 : size()); - } + const_iterator end() const { return const_iterator(*this, size()); } private: value_type min_ = 0; @@ -522,7 +537,7 @@ private: template void serialize(Archive &, unsigned); }; -/** An axis for a set of unique values. +/** Axis which maps unique single values to bins (one on one). * * The axis maps a set of values to bins, following the order of * arguments in the constructor. There is an optional overflow bin diff --git a/include/boost/histogram/detail/meta.hpp b/include/boost/histogram/detail/meta.hpp index 3299c0e3..afbbf68e 100644 --- a/include/boost/histogram/detail/meta.hpp +++ b/include/boost/histogram/detail/meta.hpp @@ -43,7 +43,7 @@ template ()), std::end(std::declval()))> struct is_sequence {}; -template struct combiner { +template struct combine { using type = typename mpl::copy_if>, @@ -51,7 +51,7 @@ template struct combiner { }; template -using combine = typename combiner::type; +using combine_t = typename combine::type; struct bool_mask_op { std::vector &b; diff --git a/include/boost/histogram/histogram_impl_dynamic.hpp b/include/boost/histogram/histogram_impl_dynamic.hpp index 1ed7e89a..52a862f4 100644 --- a/include/boost/histogram/histogram_impl_dynamic.hpp +++ b/include/boost/histogram/histogram_impl_dynamic.hpp @@ -430,20 +430,21 @@ private: }; template -inline histogram>> +inline histogram>> make_dynamic_histogram(Axes &&... axes) { return histogram>>( + detail::combine_t>>( std::forward(axes)...); } template -inline histogram>, +inline histogram>, Storage> make_dynamic_histogram_with(Axes &&... axes) { - return histogram>, - Storage>(std::forward(axes)...); + return histogram< + Dynamic, detail::combine_t>, Storage>( + std::forward(axes)...); } } // namespace histogram diff --git a/src/python/axis.cpp b/src/python/axis.cpp index 4c83b3ac..e6215161 100644 --- a/src/python/axis.cpp +++ b/src/python/axis.cpp @@ -34,6 +34,25 @@ template python::str generic_repr(const T &t) { return os.str().c_str(); } +struct generic_iterator { + generic_iterator(python::object o) : iterable(o), size(python::len(iterable)) {} + python::object next() { + if (idx == size) { + PyErr_SetString(PyExc_StopIteration, "No more items."); + python::throw_error_already_set(); + } + return iterable[idx++]; + } + python::object self() { return python::object(*this); } + python::object iterable; + unsigned idx = 0; + unsigned size = 0; +}; + +generic_iterator make_generic_iterator(python::object self) { + return generic_iterator(self); +} + template struct axis_interval_to_python { @@ -126,7 +145,7 @@ python::object category_init(python::tuple args, python::dict kwargs) { } template python::object axis_getitem(const A &a, int i) { - if (i == a.size()) { + if (i < -1 * a.uoflow() || i >= a.size() + 1 * a.uoflow()) { PyErr_SetString(PyExc_IndexError, "index out of bounds"); python::throw_error_already_set(); } @@ -195,7 +214,7 @@ struct axis_suite : public python::def_visitor> { ":param integer i: bin index" "\n:returns: bin corresponding to index", python::args("self", "i")); - // cl.def("__iter__", python::iterator()); + cl.def("__iter__", make_generic_iterator); cl.def("__repr__", generic_repr, ":returns: string representation of this axis", python::arg("self")); cl.def(python::self == python::self); @@ -270,6 +289,10 @@ void register_axis_types() { pair_int_axis_interval_to_python >(); + class_("generic_iterator", init()) + .def("__iter__", &generic_iterator::self) + .def("next", &generic_iterator::next); + class_>( "regular", "Axis for real-valued data and bins of equal width." diff --git a/test/axis_test.cpp b/test/axis_test.cpp index 6fcea645..3eaa7c7f 100644 --- a/test/axis_test.cpp +++ b/test/axis_test.cpp @@ -168,6 +168,8 @@ int main() { // axis::integer { axis::integer<> a{-1, 2}; + BOOST_TEST_EQ(a[-1].lower(), -std::numeric_limits::max()); + BOOST_TEST_EQ(a[a.size()].upper(), std::numeric_limits::max()); axis::integer<> b; BOOST_TEST_NOT(a == b); b = a; @@ -220,14 +222,13 @@ int main() { { enum { A, B, C }; test_axis_iterator(axis::regular<>(5, 0, 1, "", axis::uoflow::off), 0, 5); - test_axis_iterator(axis::regular<>(5, 0, 1, "", axis::uoflow::on), -1, 6); + test_axis_iterator(axis::regular<>(5, 0, 1, "", axis::uoflow::on), 0, 5); test_axis_iterator(axis::circular<>(5, 0, 1, ""), 0, 5); test_axis_iterator(axis::variable<>({1, 2, 3}, "", axis::uoflow::off), 0, 2); - test_axis_iterator(axis::variable<>({1, 2, 3}, "", axis::uoflow::on), -1, - 3); + test_axis_iterator(axis::variable<>({1, 2, 3}, "", axis::uoflow::on), 0, 2); test_axis_iterator(axis::integer<>(0, 4, "", axis::uoflow::off), 0, 4); - test_axis_iterator(axis::integer<>(0, 4, "", axis::uoflow::on), -1, 5); + test_axis_iterator(axis::integer<>(0, 4, "", axis::uoflow::on), 0, 4); test_axis_iterator(axis::category<>({A, B, C}, ""), 0, 3); } diff --git a/test/python_suite_test.py b/test/python_suite_test.py index dbd046e2..78255fb8 100644 --- a/test/python_suite_test.py +++ b/test/python_suite_test.py @@ -9,7 +9,9 @@ import unittest from math import pi from histogram import histogram -from histogram.axis import regular, regular_log, regular_sqrt, regular_cos, regular_pow, circular, variable, category, integer +from histogram.axis import (regular, regular_log, regular_sqrt, regular_cos, + regular_pow, circular, variable, category, + integer) import pickle import os import sys @@ -82,6 +84,8 @@ class test_regular(unittest.TestCase): for i in range(4): self.assertAlmostEqual(a[i][0], v[i]) self.assertAlmostEqual(a[i][1], v[i+1]) + self.assertEqual(a[-1][0], -float("infinity")) + self.assertEqual(a[4][1], float("infinity")) def test_iter(self): v = [1.0, 1.25, 1.5, 1.75, 2.0] @@ -118,7 +122,6 @@ class test_regular(unittest.TestCase): self.assertAlmostEqual(a[1][0], 1e1) self.assertAlmostEqual(a[1][1], 1e2) - def test_pow_transform(self): a = regular_pow(2, 1.0, 9.0, 0.5) self.assertEqual(a.index(-1), -1) @@ -237,6 +240,8 @@ class test_variable(unittest.TestCase): for i in range(2): self.assertEqual(a[i][0], v[i]) self.assertEqual(a[i][1], v[i+1]) + self.assertEqual(a[-1][0], -float("infinity")) + self.assertEqual(a[2][1], float("infinity")) def test_iter(self): v = [-0.1, 0.2, 0.3] @@ -258,44 +263,6 @@ class test_variable(unittest.TestCase): self.assertEqual(a.index(0.31), 2) self.assertEqual(a.index(10), 2) -class test_category(unittest.TestCase): - - def test_init(self): - category(1, 2, 3) - category(1, 2, 3, label="ca") - with self.assertRaises(TypeError): - category() - with self.assertRaises(TypeError): - category("1") - with self.assertRaises(TypeError): - category(1, "2") - with self.assertRaises(TypeError): - category(1, 2, label=1) - with self.assertRaises(KeyError): - category(1, 2, 3, uoflow=True) - self.assertEqual(category(1, 2, 3), - category(1, 2, 3)) - - def test_len(self): - a = category(1, 2, 3) - self.assertEqual(len(a), 3) - - def test_repr(self): - for s in ("category(1)", - "category(1, 2)", - "category(1, 2, 3)"): - self.assertEqual(str(eval(s)), s) - - def test_getitem(self): - c = 1, 2, 3 - a = category(*c) - for i in range(3): - self.assertEqual(a[i], c[i]) - - def test_iter(self): - c = [1, 2, 3] - self.assertEqual([x for x in category(*c)], c) - class test_integer(unittest.TestCase): def test_init(self): @@ -333,10 +300,14 @@ class test_integer(unittest.TestCase): a = integer(-1, 3) for i in range(4): self.assertEqual(a[i][0], v[i]) + self.assertEqual(a[-1][0], -2 ** 31 + 1) + self.assertEqual(a[4][1], 2 ** 31 - 1) def test_iter(self): - v = [x[0] for x in integer(-1, 3)] - self.assertEqual(v, [-1, 0, 1, 2]) + v = [-1, 0, 1, 2, 3] + a = integer(-1, 3) + self.assertEqual([x[0] for x in a], v[:-1]) + self.assertEqual([x[1] for x in a], v[1:]) def test_index(self): a = integer(-1, 3) @@ -349,6 +320,44 @@ class test_integer(unittest.TestCase): self.assertEqual(a.index(3), 4) self.assertEqual(a.index(4), 4) +class test_category(unittest.TestCase): + + def test_init(self): + category(1, 2, 3) + category(1, 2, 3, label="ca") + with self.assertRaises(TypeError): + category() + with self.assertRaises(TypeError): + category("1") + with self.assertRaises(TypeError): + category(1, "2") + with self.assertRaises(TypeError): + category(1, 2, label=1) + with self.assertRaises(KeyError): + category(1, 2, 3, uoflow=True) + self.assertEqual(category(1, 2, 3), + category(1, 2, 3)) + + def test_len(self): + a = category(1, 2, 3) + self.assertEqual(len(a), 3) + + def test_repr(self): + for s in ("category(1)", + "category(1, 2)", + "category(1, 2, 3)"): + self.assertEqual(str(eval(s)), s) + + def test_getitem(self): + c = 1, 2, 3 + a = category(*c) + for i in range(3): + self.assertEqual(a[i], c[i]) + + def test_iter(self): + c = [1, 2, 3] + self.assertEqual([x for x in category(*c)], c) + class test_histogram(unittest.TestCase): def test_init(self):