diff --git a/include/pybind11/eigen/matrix.h b/include/pybind11/eigen/matrix.h index ca599c954c..0773fb9834 100644 --- a/include/pybind11/eigen/matrix.h +++ b/include/pybind11/eigen/matrix.h @@ -86,6 +86,9 @@ struct EigenConformable { EigenIndex rows = 0, cols = 0; EigenDStride stride{0, 0}; // Only valid if negativestrides is false! bool negativestrides = false; // If true, do not use stride! + // A numpy stride that is not a whole number of scalars (e.g. a field of a packed structured + // array) cannot be written as an Eigen stride. If true, do not use stride! + bool badstrides = false; // NOLINTNEXTLINE(google-explicit-constructor) EigenConformable(bool fits = false) : conformable{fits} {} @@ -115,6 +118,9 @@ struct EigenConformable { if (rows == 0 || cols == 0) { return true; } + if (badstrides) { + return false; + } 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() @@ -185,19 +191,28 @@ struct EigenProps { return false; } - return {np_rows, np_cols, np_rstride, np_cstride}; + EigenConformable result{np_rows, np_cols, np_rstride, np_cstride}; + result.badstrides = !whole_scalars(a.strides(0)) || !whole_scalars(a.strides(1)); + return result; } // Otherwise we're storing an n-vector. Only one of the strides will be used, but // whichever is used, we want the (single) numpy stride value. const EigenIndex n = a.shape(0), stride = a.strides(0) / static_cast(sizeof(Scalar)); + const bool bad = !whole_scalars(a.strides(0)); + + auto make = [bad](EigenIndex r, EigenIndex c, EigenIndex s) { + EigenConformable result{r, c, s}; + result.badstrides = bad; + return result; + }; if (vector) { // Eigen type is a compile-time vector if (fixed && size != n) { return false; // Vector size mismatch } - return {rows == 1 ? 1 : n, cols == 1 ? 1 : n, stride}; + return make(rows == 1 ? 1 : n, cols == 1 ? 1 : n, stride); } if (fixed) { // The type has a fixed size, but is not a vector: abort @@ -209,12 +224,18 @@ struct EigenProps { if (cols != n) { return false; } - return {1, n, stride}; + return make(1, n, stride); } // Otherwise it's either fully dynamic, or column dynamic; both become a column vector if (fixed_rows && rows != n) { return false; } - return {n, 1, stride}; + return make(n, 1, stride); + } + + // True if a numpy byte stride is a whole number of scalars, and can therefore become an Eigen + // stride. + static bool whole_scalars(ssize_t byte_stride) { + return byte_stride % static_cast(sizeof(Scalar)) == 0; } static constexpr bool show_writeable diff --git a/tests/test_eigen_matrix.py b/tests/test_eigen_matrix.py index 9324c2a7d2..09b36c35db 100644 --- a/tests/test_eigen_matrix.py +++ b/tests/test_eigen_matrix.py @@ -221,6 +221,45 @@ def test_negative_stride_from_python(msg): ) +def test_stride_not_multiple_of_scalar_from_python(): + """A field of a packed structured array has a stride that is not a multiple of the scalar + size. Such an array must not be seen as contiguous (see #6159).""" + + structured = np.zeros(3, dtype=[("a", "f4"), ("b", "i1")]) + structured["a"] = [1.0, 2.0, 3.0] + structured["b"] = [7, 8, 9] + field = structured["a"] + assert field.strides[0] % field.itemsize != 0 + + # By value: the array is copied, the neighbouring field stays unchanged. + np.testing.assert_array_equal(m.double_col(field), [2.0, 4.0, 6.0]) + np.testing.assert_array_equal(structured["a"], [1.0, 2.0, 3.0]) + np.testing.assert_array_equal(structured["b"], [7, 8, 9]) + + # A const Ref is also allowed to copy. + wide = np.zeros(6, dtype=[("a", "f8"), ("b", "i1")]) + wide["a"] = np.arange(6.0) + assert m.get_elem_direct(wide["a"]) == 5.0 + + # A mutable Ref cannot map this layout, so it must be refused. + with pytest.raises(TypeError): + m.double_threec(field) + np.testing.assert_array_equal(structured["a"], [1.0, 2.0, 3.0]) + np.testing.assert_array_equal(structured["b"], [7, 8, 9]) + + mat = np.zeros((2, 3), dtype=[("a", "f8"), ("b", "i1")]) + mat["a"] = np.arange(6.0).reshape((2, 3)) + mat["b"] = 1 + np.testing.assert_array_equal( + m.double_mat_rm(mat["a"]), 2.0 * np.arange(6.0).reshape((2, 3)) + ) + np.testing.assert_array_equal(m.diagonal(mat["a"]), [0.0, 4.0]) + with pytest.raises(TypeError): + m.add_rm(mat["a"], 0, 0, 100.0) + np.testing.assert_array_equal(mat["a"], np.arange(6.0).reshape((2, 3))) + np.testing.assert_array_equal(mat["b"], np.ones((2, 3), dtype="i1")) + + def test_block_runtime_error_type_caster_eigen_ref_made_a_copy(): with pytest.raises(RuntimeError) as excinfo: m.block(ref, 0, 0, 0, 0)