Merge pull request #16 from EricCousineau-TRI/issue/drake_8620

eigen: Fix dtype=object shape conversion for 1D NumPy arrays for Eigen matrices

Author: EricCousineau-TRI

include/pybind11/eigen.h

@@ -223,9 +223,9 @@ template <typename props> handle eigen_array_cast(typename props::Type const &sr
     constexpr ssize_t elem_size = sizeof(typename props::Scalar);
     array a;
     using Scalar = typename props::Type::Scalar;
-    bool is_pyoject = static_cast<pybind11::detail::npy_api::constants>(npy_format_descriptor<Scalar>::value) == npy_api::NPY_OBJECT_;
+    bool is_pyobject = static_cast<pybind11::detail::npy_api::constants>(npy_format_descriptor<Scalar>::value) == npy_api::NPY_OBJECT_;
 
-    if (!is_pyoject) {
+    if (!is_pyobject) {
         if (props::vector)
             a = array({ src.size() }, { elem_size * src.innerStride() }, src.data(), base);
         else
@@ -336,7 +336,7 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
             if (dims == 1) {
                 if (Type::RowsAtCompileTime == Eigen::Dynamic)
                     value.resize(buf.shape(0), 1);
-                if (Type::ColsAtCompileTime == Eigen::Dynamic)
+                else if (Type::ColsAtCompileTime == Eigen::Dynamic)
                     value.resize(1, buf.shape(0));
 
                 for (ssize_t i = 0; i < buf.shape(0); ++i) {

tests/test_eigen.cpp

@@ -22,6 +22,10 @@
 
 using MatrixXdR = Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic, Eigen::RowMajor>;
 typedef Eigen::AutoDiffScalar<Eigen::VectorXd> ADScalar;
+
+template <typename Scalar>
+using MatrixX = Eigen::Matrix<Scalar, Eigen::Dynamic, Eigen::Dynamic>;
+
 typedef Eigen::Matrix<ADScalar, Eigen::Dynamic, 1> VectorXADScalar;
 typedef Eigen::Matrix<ADScalar, 1, Eigen::Dynamic> VectorXADScalarR;
 PYBIND11_NUMPY_OBJECT_DTYPE(ADScalar);
@@ -336,6 +340,20 @@ TEST_SUBMODULE(eigen, m) {
     m.def("iss1105_col_obj", [](VectorXADScalar) { return true; });
     m.def("iss1105_row_obj", [](VectorXADScalarR) { return true; });
 
+    // Test the shape of a matrix via `type_caster`s.
+    m.def("cpp_matrix_shape", [](const MatrixX<double>& A) {
+        return py::make_tuple(A.rows(), A.cols());
+    });
+    m.def("cpp_matrix_shape", [](const MatrixX<ADScalar>& A) {
+        return py::make_tuple(A.rows(), A.cols());
+    });
+    // TODO(eric.cousineau): Unless `dtype=ADScalar` (user-defined) and not
+    // `dtype=object`, we should kill any usages of `Eigen::Ref<>` or any
+    // const-references.
+    m.def("cpp_matrix_shape_ref", [](const Eigen::Ref<MatrixX<ADScalar>>& A) {
+        return py::make_tuple(A.rows(), A.cols());
+    });
+
     // 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)

tests/test_eigen.py

@@ -204,6 +204,18 @@ def test_eigen_passing_adscalar():
     assert "incompatible function arguments" in str(excinfo)
 
 
+def test_eigen_obj_shape():
+    # Ensure that matrices are of the same shape for dtype=object when given a 1D NumPy array.
+    # RobotLocomotion/drake#8620
+    x_f = np.array([1, -1], dtype=float)
+    shape_f = m.cpp_matrix_shape(x_f)
+    x_ad = np.array([m.AutoDiffXd(0, []), m.AutoDiffXd(0, [])], dtype=object)
+    shape_ad = m.cpp_matrix_shape(x_ad)
+    shape_ad_ref = m.cpp_matrix_shape_ref(x_ad)
+    assert shape_f == shape_ad
+    assert shape_f == shape_ad_ref
+
+
 def test_negative_stride_from_python(msg):
     """Eigen doesn't support (as of yet) negative strides. When a function takes an Eigen matrix by
     copy or const reference, we can pass a numpy array that has negative strides.  Otherwise, an