Skip to content

Migrate from pybind11 to nanobind#198

Draft
msimberg wants to merge 17 commits intoghex-org:masterfrom
msimberg:nanobind
Draft

Migrate from pybind11 to nanobind#198
msimberg wants to merge 17 commits intoghex-org:masterfrom
msimberg:nanobind

Conversation

@msimberg
Copy link
Copy Markdown
Collaborator

This is still work in progress, but functional, at least according to GHEX tests.

This was heavily assisted by LLMs (various models). All bugs are mine.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Migrates the project’s Python bindings from pybind11 to nanobind, updating the C++ binding layer, CMake integration, and CI/test setup accordingly.

Changes:

  • Switch Python binding implementation from pybind11 APIs/macros to nanobind equivalents across structured/unstructured bindings.
  • Update build configuration to locate and build against nanobind (CMake + pyproject build requirements).
  • Adjust test/CI workflows and packaging metadata to reflect the new binding backend.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/bindings/python/CMakeLists.txt Uses Python_EXECUTABLE and refactors test target dependencies.
pyproject.toml Replaces pybind11 build requirement with nanobind.
include/ghex/structured/regular/halo_generator.hpp Adds missing <vector> include.
cmake/ghex_python.cmake Switches to FindPython + nanobind discovery via python module + CMake config.
bindings/python/src/ghex/pyghex/init.py Updates documentation comment to nanobind.
bindings/python/src/ghex/CMakeLists.txt Adds pyghex_files target to refresh Python files for tests.
bindings/python/src/_pyghex/unstructured/pattern.cpp Ports bindings from pybind11 to nanobind.
bindings/python/src/_pyghex/unstructured/halo_generator.cpp Ports bindings from pybind11 to nanobind; adjusts constructors.
bindings/python/src/_pyghex/unstructured/field_descriptor.cpp Reworks buffer/ndarray handling using nanobind::ndarray.
bindings/python/src/_pyghex/unstructured/domain_descriptor.cpp Ports bindings and adds NB v1/v2 constructor handling.
bindings/python/src/_pyghex/unstructured/communication_object.cpp Ports bindings + CUDA stream extraction to nanobind.
bindings/python/src/_pyghex/structured/regular/pattern.cpp Ports bindings from pybind11 to nanobind.
bindings/python/src/_pyghex/structured/regular/halo_generator.cpp Ports bindings; adjusts __call__ return handling for nanobind.
bindings/python/src/_pyghex/structured/regular/field_descriptor.cpp Reworks buffer/ndarray handling using nanobind::ndarray.
bindings/python/src/_pyghex/structured/regular/domain_descriptor.cpp Ports bindings from pybind11 to nanobind.
bindings/python/src/_pyghex/structured/regular/communication_object.cpp Ports bindings from pybind11 to nanobind.
bindings/python/src/_pyghex/register_class.hpp Updates helper to return nanobind::class_ and properties.
bindings/python/src/_pyghex/py_dtype_to_cpp_name.cpp Reimplements dtype mapping using nanobind + NumPy dtype canonicalization.
bindings/python/src/_pyghex/mpi_comm_shim.hpp Switches shim API from pybind11 to nanobind.
bindings/python/src/_pyghex/mpi_comm_shim.cpp Ports MPI shim registration and conversion logic to nanobind.
bindings/python/src/_pyghex/module.cpp Replaces PYBIND11_MODULE with NB_MODULE and updates registrations.
bindings/python/src/_pyghex/context_shim.cpp Ports context bindings to nanobind.
bindings/python/src/_pyghex/config.cpp Ports config bindings to nanobind and updates version reporting.
bindings/python/src/_pyghex/CMakeLists.txt Builds Python extension via nanobind_add_module.
README.md Updates dependency list to Nanobind.
.github/workflows/test_pip.yml Removes pybind11 system package install in pip workflow.
.github/workflows/CI.yml Replaces python3-pybind11 with nanobind-dev in CI apt dependencies.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return MPI_Comm_f2c(o.attr("py2f")().cast<MPI_Fint>());
if (!nanobind::hasattr(o, "py2f"))
throw nanobind::type_error("Argument must be `mpi4py.MPI.Comm`");
return MPI_Comm_f2c(nanobind::cast<MPI_Fint>(o.attr("py2f")()));
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

convert_to_mpi_comm does not actually call py2f(). o.attr("py2f") returns the bound method object, so casting it to MPI_Fint will fail at runtime. Call the method and cast its result (i.e., invoke py2f before converting to MPI_Comm).

Suggested change
return MPI_Comm_f2c(nanobind::cast<MPI_Fint>(o.attr("py2f")()));
auto py2f_method = o.attr("py2f");
auto f_comm_obj = py2f_method();
MPI_Fint f_comm = nanobind::cast<MPI_Fint>(f_comm_obj);
return MPI_Comm_f2c(f_comm);

Copilot uses AI. Check for mistakes.
Comment on lines +62 to 69
const auto protocol_version = nanobind::cast<std::size_t>(cuda_stream_protocol[0]);
if (protocol_version == 0)
{
std::stringstream error;
error << "Expected `__cuda_stream__` protocol version 0, but got "
<< protocol_version;
throw pybind11::type_error(error.str());
throw nanobind::type_error(error.str().c_str());
}
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The __cuda_stream__ protocol version check is inverted: version 0 is the expected value per the CUDA stream protocol, but the code throws when protocol_version == 0. This will reject valid stream objects. Flip the condition so that only non-zero protocol versions raise an error.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@philip-paul-mueller this note looks very legit. Can you comment if it's right to complain or if the error message itself is wrong?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed by @philip-paul-mueller in #202. I'll apply the change here as well.

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.

2 participants