pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

[BUG]: Sparse Eigen type caster assumes that the index and value arrays are mutable

Open tttapa opened this issue 1 year ago • 0 comments

Required prerequisites

  • [X] Make sure you've read the documentation. Your issue may be addressed there.
  • [X] Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
  • [X] Consider asking first in the Gitter chat room or in a Discussion.

What version (or hash if on master) of pybind11 are you using?

2.13.6

Problem description

The type caster for sparse matrices assumes that the arrays inside of the csc_matrix class are mutable. This is not necessarily the case (for example, they could be initialized by Eigen::Ref<const Eigen::VectorXi> casted to a NumPy array).

https://github.com/pybind/pybind11/blob/75e48c5f959b4f0a49d8c664e059b6fb4b497102/include/pybind11/eigen/matrix.h#L680-L687

Reproducible example code

    Eigen::VectorXd values(2);
    Eigen::VectorXi inner_idx(2), outer_ptr(3);
    values << 11, 22;
    inner_idx << 0, 1;
    outer_ptr << 0, 1, 2;
    auto csc_array = py::module_::import("scipy.sparse").attr("csc_array");
#if 1
    using const_vec_i_ref = Eigen::Ref<const Eigen::VectorXi>;
    using const_vec_d_ref = Eigen::Ref<const Eigen::VectorXd>;
    auto matrix_args      = py::make_tuple(const_vec_d_ref{values}, const_vec_i_ref{inner_idx},
                                           const_vec_i_ref{outer_ptr}); // does not work
#else
    auto matrix_args = py::make_tuple(values, inner_idx, outer_ptr); // works
#endif
    auto shape        = ("shape"_a = py::make_tuple(2, 2));
    auto matrix       = csc_array(std::move(matrix_args), std::move(shape));
    auto eigen_matrix = py::cast<Eigen::SparseMatrix<double>>(matrix);
    //                      ↑ ValueError: array is not writeable
    std::cout << eigen_matrix << std::endl;

Is this a regression? Put the last known working version here if it is.

Not a regression

tttapa avatar Nov 04 '24 15:11 tttapa