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

Assess switch from pybind11 to nanobind #2712

Closed
garth-wells opened this issue Jul 6, 2023 · 15 comments
Closed

Assess switch from pybind11 to nanobind #2712

garth-wells opened this issue Jul 6, 2023 · 15 comments
Labels
proposal Suggested change or addition

Comments

@garth-wells
Copy link
Member

There may be benefits to switching from pybind11 to nanobind (https://github.com/wjakob/nanobind).

This will require a careful assessment, and the blockers resolved.

@garth-wells garth-wells added the proposal Suggested change or addition label Jul 6, 2023
@chrisrichardson
Copy link
Contributor

Known issues:

  • No complex number support (yet) in nanobind
  • No support for const pointers, e.g. std::shared_ptr<const Mesh> needs to be cast to std::shared_ptr<Mesh> before being used in nanobind - this will require a lot of new lambda functions in the wrappers.

@garth-wells
Copy link
Member Author

See wjakob/nanobind#292 on support for complex.

@chrisrichardson
Copy link
Contributor

Const is still a pain

@wjakob
Copy link

wjakob commented Sep 20, 2023

I just saw this issue because of a mention in the nanobind repo. Just to clarify: pybind11 handles shared_ptr<const...> and nanobind does not?

(If so, note that neither of these two frameworks honors const-ness, so this will likely not do what you want even for the pybind11 side.)

@chrisrichardson
Copy link
Contributor

chrisrichardson commented Sep 20, 2023

@wjakob. Yes. It is just an interfacing and convenience issue. Clearly python does not generally care about const (though I guess numpy has some idea).
The problem is we are wrapping functions with signatures including e.g. vector<vector<std:shared_ptr<const Foo>>> as input args. Now we have to unwrap/copy everything and write a lambda. Not impossible, just more laborious. pybind seems to conveniently just ignore the const.

@wjakob
Copy link

wjakob commented Sep 22, 2023

The shared_ptr issue should now be fixed on the master branch: wjakob/nanobind@21ee32b

@wjakob
Copy link

wjakob commented Oct 3, 2023

std::complex<T> is supported as of the latest release.

@garth-wells
Copy link
Member Author

std::complex<T> is supported as of the latest release.

Thanks @wjakob. Does nb::ndarray support complex floats?

@chrisrichardson
Copy link
Contributor

I've updated the branch https://github.com/FEniCS/dolfinx/tree/nanobind to sync with main.
I think shared_ptr may be now OK. Complex numbers are in nanobind for STL, but we still need ndarray to support std::complex.

@wjakob
Copy link

wjakob commented Oct 13, 2023

Another user has made a PR about this exact feature ;) -> wjakob/nanobind#319 (not sure if it's related to FEniCS)

@chrisrichardson
Copy link
Contributor

Thanks for the heads-up @wjakob. I think they are looking for alien life (SETI Institute), which is a bit outside of our domain.

@luigifcruz
Copy link

I mean, FEniCS can be useful for radio astronomy too! 😄

Just as a clarification, my PR (wjakob/nanobind#319) was written for the Python bindings of BLADE, a GPU-accelerated software being used as the DSP-backend on the Allen Telescope Array for astronomy and SETI purposes.

@wjakob
Copy link

wjakob commented Oct 18, 2023

Thanks to @luigifcruz, this is now merged 🎉 . I just pushed out nanobind 1.7.0 with this change.

@garth-wells
Copy link
Member Author

Thanks @luigifcruz, thanks @wjakob!

@garth-wells
Copy link
Member Author

Switch (almost?) complete in #2820. Can be merged once new nanobind release made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Suggested change or addition
Projects
No open projects
Development

No branches or pull requests

4 participants