Skip to content

Commit

Permalink
Fix arrays with zero-size dimensions (#4038)
Browse files Browse the repository at this point in the history
When converting an array to an Eigen matrix, ignore the strides if any
dimension size is 0. If the array is empty, the strides aren't relevant,
and especially numpy ≥ 1.23 explicitly sets the strides to 0 in this
case. (See numpy commit dd5ab7b11520.)

Update tests to verify that this works, and continues to work.
  • Loading branch information
mwoehlke-kitware authored Jun 29, 2022
1 parent 374a5b0 commit 479e9a5
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 6 deletions.
14 changes: 10 additions & 4 deletions include/pybind11/eigen.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,16 @@ struct EigenConformable {
bool stride_compatible() const {
// To have compatible strides, we need (on both dimensions) one of fully dynamic strides,
// matching strides, or a dimension size of 1 (in which case the stride value is
// irrelevant)
return !negativestrides
&& (props::inner_stride == Eigen::Dynamic || props::inner_stride == stride.inner()
|| (EigenRowMajor ? cols : rows) == 1)
// irrelevant). Alternatively, if any dimension size is 0, the strides are not relevant
// (and numpy ≥ 1.23 sets the strides to 0 in that case, so we need to check explicitly).
if (negativestrides) {
return false;
}
if (rows == 0 || cols == 0) {
return true;
}
return (props::inner_stride == Eigen::Dynamic || props::inner_stride == stride.inner()
|| (EigenRowMajor ? cols : rows) == 1)
&& (props::outer_stride == Eigen::Dynamic || props::outer_stride == stride.outer()
|| (EigenRowMajor ? rows : cols) == 1);
}
Expand Down
7 changes: 5 additions & 2 deletions tests/test_eigen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,13 @@ TEST_SUBMODULE(eigen, m) {
},
py::arg{}.noconvert());

// test_issue738
// Issue #738: 1xN or Nx1 2D matrices were neither accepted nor properly copied with an
// test_issue738, test_zero_length
// Issue #738: 1×N or N×1 2D matrices were neither accepted nor properly copied with an
// incompatible stride value on the length-1 dimension--but that should be allowed (without
// requiring a copy!) because the stride value can be safely ignored on a size-1 dimension.
// Similarly, 0×N or N×0 matrices were not accepted--again, these should be allowed since
// they contain no data. This particularly affects numpy ≥ 1.23, which sets the strides to
// 0 if any dimension size is 0.
m.def("iss738_f1",
&adjust_matrix<const Eigen::Ref<const Eigen::MatrixXd> &>,
py::arg{}.noconvert());
Expand Down
7 changes: 7 additions & 0 deletions tests/test_eigen.py
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,13 @@ def test_issue738():
)


@pytest.mark.parametrize("func", [m.iss738_f1, m.iss738_f2])
@pytest.mark.parametrize("sizes", [(0, 2), (2, 0)])
def test_zero_length(func, sizes):
"""Ignore strides on a length-0 dimension (even if they would be incompatible length > 1)"""
assert np.all(func(np.zeros(sizes)) == np.zeros(sizes))


def test_issue1105():
"""Issue 1105: 1xN or Nx1 input arrays weren't accepted for eigen
compile-time row vectors or column vector"""
Expand Down

0 comments on commit 479e9a5

Please sign in to comment.