Skip to content

Commit

Permalink
Fix 2D Nx1/1xN inputs to eigen dense vector args
Browse files Browse the repository at this point in the history
This fixes a bug introduced in b68959e
when passing in a two-dimensional, but conformable, array as the value
for a compile-time Eigen vector (such as VectorXd or RowVectorXd).  The
commit switched to using numpy to copy into the eigen data, but this
broke the described case because numpy refuses to broadcast a (N,1)
into a (N).

This commit fixes it by squeezing the input array whenever the output
array is 1-dimensional, which will let the problematic case through.
(This shouldn't squeeze inappropriately as dimension compatibility is
already checked for conformability before getting to the copy code).
  • Loading branch information
jagerman committed Oct 12, 2017
1 parent 7672292 commit 6a81dbb
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
1 change: 1 addition & 0 deletions include/pybind11/eigen.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
value = Type(fits.rows, fits.cols);
auto ref = reinterpret_steal<array>(eigen_ref_array<props>(value));
if (dims == 1) ref = ref.squeeze();
else if (ref.ndim() == 1) buf = buf.squeeze();

int result = detail::npy_api::get().PyArray_CopyInto_(ref.ptr(), buf.ptr());

Expand Down
7 changes: 7 additions & 0 deletions tests/test_eigen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,13 @@ TEST_SUBMODULE(eigen, m) {
m.def("iss738_f1", &adjust_matrix<const Eigen::Ref<const Eigen::MatrixXd> &>, py::arg().noconvert());
m.def("iss738_f2", &adjust_matrix<const Eigen::Ref<const Eigen::Matrix<double, -1, -1, Eigen::RowMajor>> &>, py::arg().noconvert());

// test_issue1105
// Issue #1105: when converting from a numpy two-dimensional (Nx1) or (1xN) value into a dense
// eigen Vector or RowVector, the argument would fail to load because the numpy copy would fail:
// numpy won't broadcast a Nx1 into a 1-dimensional vector.
m.def("iss1105_col", [](Eigen::VectorXd) { return true; });
m.def("iss1105_row", [](Eigen::RowVectorXd) { return true; });

// test_named_arguments
// Make sure named arguments are working properly:
m.def("matrix_multiply", [](const py::EigenDRef<const Eigen::MatrixXd> A, const py::EigenDRef<const Eigen::MatrixXd> B)
Expand Down
15 changes: 15 additions & 0 deletions tests/test_eigen.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,21 @@ def test_issue738():
assert np.all(m.iss738_f2(np.array([[1.], [2], [3]])) == np.array([[1.], [12], [23]]))


def test_issue1105():
"""Issue 1105: 1xN or Nx1 input arrays weren't accepted for eigen
compile-time row vectors or column vector"""
assert m.iss1105_row(np.ones((1, 7)))
assert m.iss1105_col(np.ones((7, 1)))

# These should still fail (incompatible dimensions):
with pytest.raises(TypeError) as excinfo:
m.iss1105_row(np.ones((7, 1)))
assert "incompatible function arguments" in str(excinfo)
with pytest.raises(TypeError) as excinfo:
m.iss1105_col(np.ones((1, 7)))
assert "incompatible function arguments" in str(excinfo)


def test_custom_operator_new():
"""Using Eigen types as member variables requires a class-specific
operator new with proper alignment"""
Expand Down

0 comments on commit 6a81dbb

Please sign in to comment.