From df4a6e269a7d126f4b647719ce460e6bea8f2ef2 Mon Sep 17 00:00:00 2001 From: Aman Goel Date: Mon, 13 Sep 2021 09:37:24 +0530 Subject: [PATCH] feat: add boilerplate sub implementation (#636) * feat: add boilerplate sub implementation * feat: subtraction for Histograms * feat: use signed integers for integer storages * tests: add tests for subtraction * feat: support subtraction between WeightedSum views * docs: update CHANGELOG Co-authored-by: Henry Schreiner --- docs/CHANGELOG.md | 10 ++++++ include/bh_python/register_histogram.hpp | 3 ++ include/bh_python/storage.hpp | 10 +++--- src/boost_histogram/_internal/hist.py | 17 +++++++++ src/boost_histogram/_internal/view.py | 4 +-- tests/conftest.py | 16 ++++++++- tests/test_histogram.py | 44 ++++++++++++++++++++++++ tests/test_views.py | 22 ++++++++---- 8 files changed, 112 insertions(+), 14 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 5c39ce4e..d4929659 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -6,14 +6,24 @@ #### User changes * Python 3.10 officially supported, with wheels. +* Support subtraction on histograms [#636][] #### Bug fixes * Support custom setters on AxesTuple subclasses. [#627][] * Throw an error when an AxesTuple setter is the wrong length (inspired by zip strict in Python 3.10) [#627][] * Fix error thrown on comparison with axis and non-axis object [#631][] +* Static typing no longer thinks `storage=` is required [#604][] +#### Developer changes +* Support NumPy 1.21 for static type checking [#625][] +* Use newer Boost 1.77 and Boost.Histogram 1.77+1 [#594][] + +[#594]: https://github.com/scikit-hep/boost-histogram/pull/594 +[#604]: https://github.com/scikit-hep/boost-histogram/pull/604 +[#625]: https://github.com/scikit-hep/boost-histogram/pull/625 [#627]: https://github.com/scikit-hep/boost-histogram/pull/627 [#631]: https://github.com/scikit-hep/boost-histogram/pull/631 +[#636]: https://github.com/scikit-hep/boost-histogram/pull/636 ### Version 1.1.0 diff --git a/include/bh_python/register_histogram.hpp b/include/bh_python/register_histogram.hpp index 68ab7a78..acd41b47 100644 --- a/include/bh_python/register_histogram.hpp +++ b/include/bh_python/register_histogram.hpp @@ -97,6 +97,9 @@ auto register_histogram(py::module& m, const char* name, const char* desc) { def_optionally(hist, bh::detail::has_operator_rmul{}, py::self *= py::self); + def_optionally(hist, + bh::detail::has_operator_rsub{}, + py::self -= py::self); #ifdef __clang__ #pragma GCC diagnostic pop #endif diff --git a/include/bh_python/storage.hpp b/include/bh_python/storage.hpp index 75646e0c..a4027994 100644 --- a/include/bh_python/storage.hpp +++ b/include/bh_python/storage.hpp @@ -22,8 +22,8 @@ namespace storage { // Names match Python names -using int64 = bh::dense_storage; -using atomic_int64 = bh::dense_storage>; +using int64 = bh::dense_storage; +using atomic_int64 = bh::dense_storage>; using double_ = bh::dense_storage; using unlimited = bh::unlimited_storage<>; using weight = bh::dense_storage>; @@ -80,7 +80,7 @@ template void save(Archive& ar, const storage::atomic_int64& s, unsigned /* version */) { // We cannot view the memory as a numpy array, because the internal layout of // std::atomic is undefined. So no reinterpret_casts are allowed. - py::array_t a(static_cast(s.size())); + py::array_t a(static_cast(s.size())); auto in_ptr = s.begin(); auto out_ptr = a.mutable_data(); @@ -191,7 +191,7 @@ struct type_caster { auto ptr = PyNumber_Long(src.ptr()); if(!ptr) return false; - value = PyLong_AsUnsignedLongLong(ptr); + value = PyLong_AsLongLong(ptr); Py_DECREF(ptr); return !PyErr_Occurred(); } @@ -199,7 +199,7 @@ struct type_caster { static handle cast(storage::atomic_int64::value_type src, return_value_policy /* policy */, handle /* parent */) { - return PyLong_FromUnsignedLongLong(src.value()); + return PyLong_FromLongLong(src.value()); } }; } // namespace detail diff --git a/src/boost_histogram/_internal/hist.py b/src/boost_histogram/_internal/hist.py index 0cda0b9a..ad30bd3e 100644 --- a/src/boost_histogram/_internal/hist.py +++ b/src/boost_histogram/_internal/hist.py @@ -337,6 +337,23 @@ def __radd__( ) -> H: return self + other + def __sub__( + self: H, other: Union["Histogram", "np.typing.NDArray[Any]", float] + ) -> H: + result = self.copy(deep=False) + return result.__isub__(other) + + def __isub__( + self: H, other: Union["Histogram", "np.typing.NDArray[Any]", float] + ) -> H: + if isinstance(other, (int, float)) and other == 0: + return self + self._compute_inplace_op("__isub__", other) + + self.axes = self._generate_axes_() + + return self + # If these fail, the underlying object throws the correct error def __mul__( self: H, other: Union["Histogram", "np.typing.NDArray[Any]", float] diff --git a/src/boost_histogram/_internal/view.py b/src/boost_histogram/_internal/view.py index 788704bf..b06ab245 100644 --- a/src/boost_histogram/_internal/view.py +++ b/src/boost_histogram/_internal/view.py @@ -130,14 +130,14 @@ def __array_ufunc__( # Addition of two views if input_0.dtype == input_1.dtype: - if ufunc in {np.add}: + if ufunc in {np.add, np.subtract}: ufunc( input_0["value"], input_1["value"], out=result["value"], **kwargs, ) - ufunc( + np.add( input_0["variance"], input_1["variance"], out=result["variance"], diff --git a/tests/conftest.py b/tests/conftest.py index c43903a7..b4888fa4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,6 @@ import pytest -import boost_histogram # noqa: F401 +import boost_histogram as bh @pytest.fixture(params=(False, True), ids=("no_growth", "growth")) @@ -29,3 +29,17 @@ def flow(request): ) def metadata(request): return request.param + + +@pytest.fixture( + params=( + bh.storage.Double, + bh.storage.Int64, + bh.storage.AtomicInt64, + bh.storage.Weight, + bh.storage.Unlimited, + ), + ids=("Double", "Int64", "AtomicInt64", "Weight", "Unlimited"), +) +def count_storage(request): + return request.param diff --git a/tests/test_histogram.py b/tests/test_histogram.py index fffebd0f..7478cd01 100644 --- a/tests/test_histogram.py +++ b/tests/test_histogram.py @@ -403,6 +403,50 @@ def test_add_2d_w(flow): assert h[bh.tag.at(i), bh.tag.at(j)] == 2 * m[i][j] +def test_sub_2d(flow, count_storage): + + h0 = bh.Histogram( + bh.axis.Integer(-1, 2, underflow=flow, overflow=flow), + bh.axis.Regular(4, -2, 2, underflow=flow, overflow=flow), + storage=count_storage(), + ) + + h0.fill(-1, -2) + h0.fill(-1, -1) + h0.fill(0, 0) + h0.fill(0, 1) + h0.fill(1, 0) + h0.fill(3, -1) + h0.fill(0, -3) + + m = h0.values(flow=True).copy() + + if count_storage not in {bh.storage.AtomicInt64, bh.storage.Weight}: + h = h0.copy() + h -= h0 + assert h.values(flow=True) == approx(m * 0) + + h -= h0 + assert h.values(flow=True) == approx(-m) + + h2 = h0 - (h0 + h0 + h0) + assert h2.values(flow=True) == approx(-2 * m) + + h3 = h0 - h0.view(flow=True) * 4 + assert h3.values(flow=True) == approx(-3 * m) + + h4 = h0.copy() + h4 -= h0.view(flow=True) * 5 + assert h4.values(flow=True) == approx(-4 * m) + + h5 = h0.copy() + h5 -= 2 + assert h5.values(flow=True) == approx(m - 2) + + h6 = h0 - 3 + assert h6.values(flow=True) == approx(m - 3) + + def test_repr(): hrepr = """Histogram( Regular(3, 0, 1), diff --git a/tests/test_views.py b/tests/test_views.py index c2f5c746..95c6cb95 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -65,9 +65,14 @@ def test_view_add(v): assert_allclose(v2.value, [2, 5, 4, 3]) assert_allclose(v2.variance, [4, 7, 6, 5]) - v += 2 - assert_allclose(v.value, [2, 5, 4, 3]) - assert_allclose(v.variance, [4, 7, 6, 5]) + v2 = v.copy() + v2 += 2 + assert_allclose(v2.value, [2, 5, 4, 3]) + assert_allclose(v2.variance, [4, 7, 6, 5]) + + v2 = v + v + assert_allclose(v2.value, v.value * 2) + assert_allclose(v2.variance, v.variance * 2) def test_view_sub(v): @@ -83,9 +88,14 @@ def test_view_sub(v): assert_allclose(v2.value, [1, -2, -1, 0]) assert_allclose(v2.variance, [1, 4, 3, 2]) - v -= 2 - assert_allclose(v.value, [-2, 1, 0, -1]) - assert_allclose(v.variance, [4, 7, 6, 5]) + v2 = v.copy() + v2 -= 2 + assert_allclose(v2.value, [-2, 1, 0, -1]) + assert_allclose(v2.variance, [4, 7, 6, 5]) + + v2 = v - v + assert_allclose(v2.value, [0, 0, 0, 0]) + assert_allclose(v2.variance, v.variance * 2) def test_view_unary(v):