Skip to content

Commit

Permalink
feat: add boilerplate sub implementation (#636)
Browse files Browse the repository at this point in the history
* 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 <henryschreineriii@gmail.com>
  • Loading branch information
amangoel185 and henryiii authored Sep 13, 2021
1 parent 802a87e commit df4a6e2
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 14 deletions.
10 changes: 10 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions include/bh_python/register_histogram.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ auto register_histogram(py::module& m, const char* name, const char* desc) {
def_optionally(hist,
bh::detail::has_operator_rmul<histogram_t, histogram_t>{},
py::self *= py::self);
def_optionally(hist,
bh::detail::has_operator_rsub<histogram_t, histogram_t>{},
py::self -= py::self);
#ifdef __clang__
#pragma GCC diagnostic pop
#endif
Expand Down
10 changes: 5 additions & 5 deletions include/bh_python/storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
namespace storage {

// Names match Python names
using int64 = bh::dense_storage<uint64_t>;
using atomic_int64 = bh::dense_storage<bh::accumulators::count<uint64_t, true>>;
using int64 = bh::dense_storage<int64_t>;
using atomic_int64 = bh::dense_storage<bh::accumulators::count<int64_t, true>>;
using double_ = bh::dense_storage<double>;
using unlimited = bh::unlimited_storage<>;
using weight = bh::dense_storage<accumulators::weighted_sum<double>>;
Expand Down Expand Up @@ -80,7 +80,7 @@ template <class Archive>
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<std::uint64_t> a(static_cast<py::ssize_t>(s.size()));
py::array_t<std::int64_t> a(static_cast<py::ssize_t>(s.size()));

auto in_ptr = s.begin();
auto out_ptr = a.mutable_data();
Expand Down Expand Up @@ -191,15 +191,15 @@ struct type_caster<storage::atomic_int64::value_type> {
auto ptr = PyNumber_Long(src.ptr());
if(!ptr)
return false;
value = PyLong_AsUnsignedLongLong(ptr);
value = PyLong_AsLongLong(ptr);
Py_DECREF(ptr);
return !PyErr_Occurred();
}

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
Expand Down
17 changes: 17 additions & 0 deletions src/boost_histogram/_internal/hist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions src/boost_histogram/_internal/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
16 changes: 15 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
@@ -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"))
Expand Down Expand Up @@ -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
44 changes: 44 additions & 0 deletions tests/test_histogram.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
22 changes: 16 additions & 6 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down

0 comments on commit df4a6e2

Please sign in to comment.