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

Initial support for the Eigen library #120

Merged
merged 6 commits into from
Feb 1, 2023
Merged

Initial support for the Eigen library #120

merged 6 commits into from
Feb 1, 2023

Conversation

wjakob
Copy link
Owner

@wjakob wjakob commented Feb 1, 2023

This commit adds custom casters for the Eigen linear algebra library. It uses the existing nb_tensor infrastructure to implement a conversion between Eigen types and NumPy arrays that currently supports:

  • Eigen::Matrix/Array<..> and derived types (row/column vectors)
  • Eigen::Map<..>
  • Eigen::Ref<..>
  • Expression templates (CwiseBinaryOp, etc.)

There are a few limitations:

  • Eigen tensors are not supported yet.
  • Sparse matrix types are not supported yet.

The step responsible for checking whether a tensor has the right strides
to invoke a C++ routine was incorrect and always accepted C-style NumPy
arrays (because they leave the 'stride' field set to NULL).

The whole conversion logic will be more heavily tested in a subsequent
commit that adds a type caster for Eigen arrays.
- Prefer the buffer protocol over DLPack when converting C++ tensors
  into NumPy arrays. This makes it possible to wrap C++ memory through
  arrays that have their 'write' bit still enabled (NumPy is somewhat
  conservative here and disables the write bit when the conversion is
  done via DLPack).

- Simplified the implementation of the internally used `nb_tensor`
  buffer protocol wrapper. This change requires a nanobind ABI bump.

- Copy the contents when `rv_policy::copy/move` is specified
  to the tensor caster.
@wjakob
Copy link
Owner Author

wjakob commented Feb 1, 2023

Could be interesting for you: @ianhbell, @brettc, @froody

This commit adds custom casters for the Eigen linear algebra library. It
uses the existing `nb_tensor` infrastructure to implement a conversion
between Eigen types and NumPy arrays that currently supports:

- Eigen::Matrix/Array<..> and derived types (row/column vectors)
- Eigen::Map<..>
- Eigen::Ref<..>
- Expression templates (CwiseBinaryOp, etc.)

There are a few limitations:

- Eigen tensors are not supported yet.
- Sparse matrix types are not supported yet.
@wjakob wjakob merged commit 8adabe3 into master Feb 1, 2023
@wjakob wjakob deleted the eigen branch February 1, 2023 10:56
@ianhbell
Copy link

ianhbell commented Feb 1, 2023

I am very happy about this addition; but doesn't this cut against the explicit design goals of nanobind to be small and targeted?

@wjakob
Copy link
Owner Author

wjakob commented Feb 1, 2023

@ianhbell: the issue with pybind11 that motivated the creation of this library was that a number of relatively niche features added overheads to the core, reducing performance for everyone (even for users that did not use or want those features). Adding features to either library when they are in the form of self-contained/opt-in components seems perfectly fine in comparison.

That said, the implementation of the Eigen support layer proposed in this PR is relatively "nano" compared to the one in pybind11.

@ianhbell
Copy link

ianhbell commented Feb 1, 2023

Agreed; opt-in is always the way to go. I'm looking forward to playing with the Eigen support in nanobind.

@brettc
Copy link

brettc commented Feb 2, 2023

Thanks @wjakob, this does look interesting. I have settled on using xtensor for my current projects, but this gives some hints on how to integrate it.

My apologies for letting the PR stagnate. I'm still keen to push it through, but it needs more thought and my TODO keeps growing.

@wjakob
Copy link
Owner Author

wjakob commented Feb 2, 2023

My apologies for letting the PR stagnate. I'm still keen to push it through, but it needs more thought and my TODO keeps growing.

Can you check with the latest version to see if it is needed? I moved the NumPy tensor exchange back onto the buffer protocol.

@brettc
Copy link

brettc commented Feb 2, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants