Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Support Eigen::Map<Eigen::SparseMatrix> #782

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 104 additions & 3 deletions include/nanobind/eigen/sparse.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,116 @@ template <typename T> struct type_caster<T, enable_if_t<is_eigen_sparse_matrix_v
/// Caster for Eigen::Map<Eigen::SparseMatrix>, still needs to be implemented.
template <typename T>
struct type_caster<Eigen::Map<T>, enable_if_t<is_eigen_sparse_matrix_v<T>>> {
using Scalar = typename T::Scalar;
using StorageIndex = typename T::StorageIndex;
using Index = typename T::Index;
using SparseMap = Eigen::Map<T>;
using Map = Eigen::Map<T>;
using SparseMatrixCaster = type_caster<T>;
static constexpr auto Name = SparseMatrixCaster::Name;
static constexpr bool RowMajor = T::IsRowMajor;

using ScalarNDArray = ndarray<numpy, Scalar, shape<-1>>;
using StorageIndexNDArray = ndarray<numpy, StorageIndex, shape<-1>>;

using ScalarCaster = make_caster<ScalarNDArray>;
using StorageIndexCaster = make_caster<StorageIndexNDArray>;

static constexpr auto Name = const_name<RowMajor>("scipy.sparse.csr_matrix[",
"scipy.sparse.csc_matrix[")
+ make_caster<Scalar>::Name + const_name("]");

template <typename T_> using Cast = Map;
template <typename T_> static constexpr bool can_cast() { return true; }

bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept = delete;
ScalarCaster data_caster;
StorageIndexCaster indices_caster, indptr_caster;
Index rows, cols, nnz;

bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept
{
object obj = borrow(src);
try {
object matrix_type = module_::import_("scipy.sparse").attr(RowMajor ? "csr_matrix" : "csc_matrix");
if (!obj.type().is(matrix_type))
obj = matrix_type(obj);
} catch (const python_error &) {
Comment on lines +186 to +189
Copy link

@bilmes bilmes Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a comment on lines 178-180. I might not be understanding this precisely, but doesn't this bit of code attempt to convert (line 180) obj to a sparse matrix if it is not already one? If so, this might cause issues where an overloaded routine that could take either a dense or sparse matrix, if the sparse matrix routine is considered first, anything that can be converted to a sparse matrix via csr_matrix (such as an int, float, or dense matrix) would then be converted to a sparse matrix and that's probaby not desirable. A simple fix would be to just fail (return false) if obj.type().is(matrix_type) is not true.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, another minor comment on the above. If it is the case that you do end up returning false if !obj.type().is(matrix_type) then the catch block of the exception I'd guess should probably re-throw the exception since it probably means something is wrong other than that the object is not a sparse matrix (e.g., an import or some other error). I.e., the return of true/false should probably (unless I don't understand things) only happen if there are no errors. Similarly, in the below, when you are returning false, this means that it passes the above type check but there is still a problem getting, say, indices, and I'd guess that would mean something is quite wrong (not well indicated by a false return) with the object and/or setup.

return false;
}
if (object data_o = obj.attr("data"); !data_caster.from_python(data_o, flags, cleanup))
return false;
ScalarNDArray& values = data_caster.value;

if (object indices_o = obj.attr("indices"); !indices_caster.from_python(indices_o, flags, cleanup))
return false;
StorageIndexNDArray& inner_indices = indices_caster.value;

if (object indptr_o = obj.attr("indptr"); !indptr_caster.from_python(indptr_o, flags, cleanup))
return false;
StorageIndexNDArray& outer_indices = indptr_caster.value;

object shape_o = obj.attr("shape"), nnz_o = obj.attr("nnz");
try {
if (len(shape_o) != 2)
return false;
rows = cast<Index>(shape_o[0]);
cols = cast<Index>(shape_o[1]);
nnz = cast<Index>(nnz_o);
} catch (const python_error &) {
return false;
}
return true;
}

static handle from_cpp(const Map &v, rv_policy policy, cleanup_list *cleanup) noexcept
{
if (!v.isCompressed()) {
PyErr_SetString(PyExc_ValueError,
"nanobind: unable to return an Eigen sparse matrix that is not in a compressed format. "
"Please call `.makeCompressed()` before returning the value on the C++ end.");
return handle();
}

object matrix_type;
try {
matrix_type = module_::import_("scipy.sparse").attr(RowMajor ? "csr_matrix" : "csc_matrix");
} catch (python_error &e) {
e.restore();
return handle();
}

static handle from_cpp(const Map &v, rv_policy policy, cleanup_list *cleanup) noexcept = delete;
const Index rows = v.rows(), cols = v.cols();
const size_t data_shape[] = { (size_t) v.nonZeros() };
const size_t outer_indices_shape[] = { (size_t) ((RowMajor ? rows : cols) + 1) };

T *src = std::addressof(const_cast<T &>(v));
object owner;
if (policy == rv_policy::move) {
src = new T(std::move(v));
owner = capsule(src, [](void *p) noexcept { delete (T *) p; });
}

ScalarNDArray data(src->valuePtr(), 1, data_shape, owner);
StorageIndexNDArray outer_indices(src->outerIndexPtr(), 1, outer_indices_shape, owner);
StorageIndexNDArray inner_indices(src->innerIndexPtr(), 1, data_shape, owner);

try {
return matrix_type(nanobind::make_tuple(
std::move(data), std::move(inner_indices), std::move(outer_indices)),
nanobind::make_tuple(rows, cols))
.release();
} catch (python_error &e) {
e.restore();
return handle();
}
};

operator Map()
{
ScalarNDArray& values = data_caster.value;
StorageIndexNDArray& inner_indices = indices_caster.value;
StorageIndexNDArray& outer_indices = indptr_caster.value;
return SparseMap(rows, cols, nnz, outer_indices.data(), inner_indices.data(), values.data());
}
};


Expand Down