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

[BUG]: cannot pass 1d matrix (numpy shape (1,1)) across nanobind as Eigen::Matrix<double,-1,-1> #544

Closed
breathe opened this issue Apr 23, 2024 · 5 comments

Comments

@breathe
Copy link

breathe commented Apr 23, 2024

Problem description

I get a type error when I try to pass a 1d ndarray to c++ function accepting Eigen::Matrix<double,-1,-1> (or Eigen::Array<double,-1-1>.

Behavior here seems unexpected to me -- I don't know if this might be too hard an issue to be fixable implementation wise ...

Thanks much!

Reproducible example code

c++

#include <nanobind/nanobind.h>
#include <nanobind/eigen/dense.h>

namespace nb = nanobind;

void processMatrix(const Eigen::Matrix<double, -1, -1> &mat)
{
    std::cout << "Received matrix: \n"
              << mat << std::endl;
}

NB_MODULE(test_native, m)
{
    m.def("process_matrix", &processMatrix, nb::arg("mat"));
}

python

    from test_native import process_matrix
    import numpy as np

    v = np.array([[1.0]])

    process_matrix(v)

    print("done")

Error

Exception has occurred: TypeError       (note: full exception trace is shown but execution is paused at: _run_module_as_main)
process_matrix(): incompatible function arguments. The following argument types are supported:
    1. process_matrix(mat: numpy.ndarray[dtype=float64, shape=(*, *), order='F']) -> None

Invoked with types: ndarray
@breathe breathe changed the title [BUG]: cannot pass 1d matrix (numpy shape 1,1) across nanobind as Eigen::Matrix<double,-1,-1> [BUG]: cannot pass 1d matrix (numpy shape (1,1)) across nanobind as Eigen::Matrix<double,-1,-1> Apr 23, 2024
@ThoreWietzke
Copy link
Contributor

please change your C++ function argument to
const Eigen::Ref<const Eigen::Matrix<double, -1, -1>>& mat
and see if that solves your issue.

I suspect that the ordering of the array is the problem, as Eigen uses a column major format and Numpy a row major format. You can also change the layout in Eigen with Eigen::RowMajor.

@wjakob
Copy link
Owner

wjakob commented Apr 24, 2024

If that is the explanation, then I'm surprised that the Eigen type caster does not make an implicit copy (which is explicitly allowed in this case.) Maybe @WKarel knows?

@breathe
Copy link
Author

breathe commented Apr 24, 2024

please change your C++ function argument to
const Eigen::Ref<const Eigen::Matrix<double, -1, -1>>& mat
and see if that solves your issue.

Interesting -- that works -- thank you very much!

I had thought that the row major vs column major ordering was implicitly handled with a copy where required. This signature also works!

void processMatrix(const Eigen::Matrix<double, -1, -1, Eigen::RowMajor> &mat)
{
    std::cout << "Received matrix: \n"
              << mat << std::endl;
}

Oddly enough, the original signature (with RowMajor on numpy side and ColMajor on Eigen side) works if you pass at least an ndarray with shape (>=2, >=2)

@WKarel
Copy link
Contributor

WKarel commented Apr 24, 2024

I have never really worked on the type_caster for plain Eigen types (but mostly on Ref and Map), and there is surely room for improvement - see also #504
As this type_caster makes a copy anyway, it should be most liberal on what to accept as nb::ndarray.

@wjakob
Copy link
Owner

wjakob commented May 22, 2024

Thanks for the report. The issue was unrelated to the Eigen<->nanobind interface and is fixed in a76cfff.

@wjakob wjakob closed this as completed May 22, 2024
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

No branches or pull requests

4 participants