dolfinx icon indicating copy to clipboard operation
dolfinx copied to clipboard

Pass custom_data through assemblers to kernel functions.

Open sclaus2 opened this issue 2 months ago • 7 comments

Add void* custom_data member to integral_data struct and pass it to assembly functions (matrix, vector, scalar, lifting). Custom_data defaults back to nullptr if no custom_data is set explicitly (with set_custom_data()). The required changes are minimal

  • Form.h: Add custom_data to integral_data, add accessors
  • assemble_matrix_impl.h: Pass custom_data through all functions
  • assemble_vector_impl.h: Pass custom_data through all functions
  • assemble_scalar_impl.h: Pass custom_data through all functions
  • Python bindings: Expose set_custom_data() and custom_data()
  • Add test_custom_data.py with comprehensive tests

This enables integration kernels to receive additional per-cell user data which is essential for example for runtime quadrature rules. With this extensions users are no longer required to completely rewrite all assembly routines for customised C-kernels.

For an example library that relies on custom_data and its integration into the assembler see

https://github.com/sclaus2/runintgen

sclaus2 avatar Dec 01 '25 14:12 sclaus2

Instead of void* and nullptr it would probably be better to use std::optional<void*>.

jhale avatar Dec 01 '25 15:12 jhale

I say probably because it's a performance critical part of the code - might need a bit of thought.

jhale avatar Dec 01 '25 15:12 jhale

The void* approach should have minimal overhead because:

  1. The kernel function pointer signature hasn't changed, it is void (kernel)(T, const T*, const T*, const U*, const int*, const uint8_t*, void*)

  2. The call site simply passes the pointer (or nullptr) kernel(..., custom_data); // Same cost whether nullptr or valid pointer, before: null_ptr

Potential overhead is:

Storing the pointer in integral_data struct (+8 bytes per integral) Passing the pointer to kernel (already in the signature)

I can run some performance benchmarks of this branch vs 0.10 release. If you have any existing performance benchmarks that you trust let me know.

sclaus2 avatar Dec 01 '25 16:12 sclaus2

I didn't explain myself very well. In modern C++ it's canonical to use std::optional rather than a pointer and nullptr to represent a value or absence of value. This also tends to wrap nicely into Python.

For example:

std::optional<void*> opt;
void* raw = opt.value_or(nullptr);
// pass raw up to C kernel

jhale avatar Dec 01 '25 17:12 jhale

How about std::any? It is also type agnostic, but type safe, and would get rid of the undesirable void* in the source. For the kernel invocation it can be cast down to void*.

schnellerhase avatar Dec 01 '25 21:12 schnellerhase

I have changed it to std::optional<void*> for now. Open for suggestions.

sclaus2 avatar Dec 01 '25 21:12 sclaus2

How about std::any? It is also type agnostic, but type safe, and would get rid of the undesirable void* in the source. For the kernel invocation it can be cast down to void*.

I don't think nanobind supports std::any.

I'm not sure about the argument regardingvoid* being undesirable. This feature is deliberately type and memory-unsafe, and this clearly signals that the user needs to be very, very careful. I've asked @sclaus2 to add something in the docstring to that effect.

jhale avatar Dec 02 '25 15:12 jhale