Move nanobind declarations to header files#4084
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the DOLFINx Python bindings to move nanobind declarations from .cpp files to header files, enabling users to declare templated bindings for custom data types. The refactoring introduces a trait-based design for NumPy dtype support and allows extension through specialization of is_custom_scalar and numpy_dtype_traits.
Changes:
- Moved template function declarations from anonymous namespaces in .cpp files to header files in
dolfinx_wrappers/directory - Renamed functions from
export_*pattern todeclare_*pattern for consistency - Refactored
numpy_dtypeto use a trait-based design that can be specialized for custom types - Added
is_custom_scalartrait tocommon/types.hto support custom scalar types - Added
#pragma oncetopycoeff.h
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
dolfinx_wrappers/geometry.h |
New header with geometry binding declarations (BoundingBoxTree, point ownership) |
dolfinx_wrappers/mesh.h |
New header with mesh binding declarations (Mesh, MeshTags, geometry functions) |
dolfinx_wrappers/la.h |
New header with linear algebra binding declarations (Vector, MatrixCSR, SuperLU) |
dolfinx_wrappers/io.h |
New header with I/O binding declarations (XDMF, VTK, VTX writers) |
dolfinx_wrappers/fem.h |
New header with FEM binding declarations (forms, functions, elements) |
dolfinx_wrappers/graph.h |
New header with graph binding declarations (AdjacencyList) |
dolfinx_wrappers/refinement.h |
New header with refinement binding declarations |
dolfinx_wrappers/petsc.h |
New header with PETSc-specific binding declarations |
dolfinx_wrappers/common.h |
New header with common utilities (scatter functions) |
dolfinx_wrappers/assemble.h |
New header with assembly function declarations |
dolfinx_wrappers/numpy_dtype.h |
Refactored to use trait-based design for extensibility |
dolfinx_wrappers/pycoeff.h |
Added #pragma once include guard |
common/types.h |
Added is_custom_scalar trait for custom scalar type support |
__init__.py |
Added get_include_dirs() helper function |
Various .cpp files |
Updated to include new headers and use declare_* functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// @param type String representation of the scalar type (e.g., "float64", | ||
| /// "float32") | ||
| template <typename T> | ||
| void declare_bbtree(nb::module_& m, const std::string& type) |
There was a problem hiding this comment.
The nb namespace alias is used in the function signature at line 35 (nb::module_& m), but it's not declared within the dolfinx_wrappers namespace. The namespace nb = nanobind; declaration should be added before using nb:: in the function signature, or the function should use the fully qualified nanobind::module_& type.
python/dolfinx/__init__.py
Outdated
| pass | ||
|
|
||
| try: | ||
| import petsc4py |
There was a problem hiding this comment.
This isn't really safe - we allow petsc4py to be installed and DOLFINx to not be compiled against it. Use has_petsc4py instead.
python/dolfinx/__init__.py
Outdated
| dirs = [get_include()] | ||
|
|
||
| try: | ||
| import mpi4py |
There was a problem hiding this comment.
This is mandatory dependency, no need to be so polite with mpi4py.
python/dolfinx/__init__.py
Outdated
| return os.path.join(os.path.dirname(d), "src") | ||
|
|
||
|
|
||
| def get_include_dirs() -> list[str]: |
python/dolfinx/wrappers/common.cpp
Outdated
| #include <dolfinx/common/log.h> | ||
| #include <dolfinx/common/timing.h> | ||
| #include <dolfinx/common/utils.h> | ||
| #include <iostream> |
|
Would be good to get this in to avoid merge hell. |
This PR allows users to declare templated binding for custom data types. For example,
Also, naming is unified to follow
declare_foofor all bindings.