Skip to content

Comments

Move nanobind declarations to header files#4084

Open
michalhabera wants to merge 16 commits intomainfrom
michal/public-declare-wrappers
Open

Move nanobind declarations to header files#4084
michalhabera wants to merge 16 commits intomainfrom
michal/public-declare-wrappers

Conversation

@michalhabera
Copy link
Contributor

@michalhabera michalhabera commented Feb 12, 2026

This PR allows users to declare templated binding for custom data types. For example,

// my_custom_binding.cpp

#include <dolfinx_wrappers/fem.h>

//...

template <>
struct dolfinx::is_custom_scalar<my_scalar_t> : std::true_type {};

template <>
struct nanobind::ndarray_traits<my_scalar_t> {
  static constexpr bool is_complex = false;
  static constexpr bool is_float = true;
  static constexpr bool is_bool = false;
  static constexpr bool is_int = false;
  static constexpr bool is_signed = true;
};

template <>
struct dolfinx_wrappers::numpy_dtype_traits<my_scalar_t> {
  static constexpr char value = 'd';
};

NB_MODULE(_custom_ext, m) {
  // ...
  dolfinx_wrappers::declare_objects<my_scalar_t>(m, "my_scalar");
  dolfinx_wrappers::declare_form<my_scalar_t>(m, "my_scalar");
}

Also, naming is unified to follow declare_foo for all bindings.

Copy link

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

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 to declare_* pattern for consistency
  • Refactored numpy_dtype to use a trait-based design that can be specialized for custom types
  • Added is_custom_scalar trait to common/types.h to support custom scalar types
  • Added #pragma once to pycoeff.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)
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
pass

try:
import petsc4py
Copy link
Member

Choose a reason for hiding this comment

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

This isn't really safe - we allow petsc4py to be installed and DOLFINx to not be compiled against it. Use has_petsc4py instead.

dirs = [get_include()]

try:
import mpi4py
Copy link
Member

Choose a reason for hiding this comment

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

This is mandatory dependency, no need to be so polite with mpi4py.

return os.path.join(os.path.dirname(d), "src")


def get_include_dirs() -> list[str]:
Copy link
Member

Choose a reason for hiding this comment

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

Where is this used?

#include <dolfinx/common/log.h>
#include <dolfinx/common/timing.h>
#include <dolfinx/common/utils.h>
#include <iostream>
Copy link
Member

Choose a reason for hiding this comment

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

Needed?

@jhale
Copy link
Member

jhale commented Feb 24, 2026

Would be good to get this in to avoid merge hell.

@michalhabera michalhabera marked this pull request as ready for review February 24, 2026 22:00
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