Pass custom_data through assemblers to kernel functions.#4013
Pass custom_data through assemblers to kernel functions.#4013
Conversation
|
Instead of |
|
I say probably because it's a performance critical part of the code - might need a bit of thought. |
|
The void* approach should have minimal overhead because:
Potential overhead is: Storing the pointer in integral_data struct (+8 bytes per integral) 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. |
|
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: |
|
How about |
|
I have changed it to std::optional<void*> for now. Open for suggestions. |
648f439 to
cf52114
Compare
Replace void* with nullptr pattern with std::optional<void*> for custom_data in the Form class and assembly functions. This is the canonical modern C++ approach for representing optional values. Changes: - Form.h: integral_data::custom_data now uses std::optional<void*> - Form.h: custom_data() returns std::optional<void*> - Form.h: set_custom_data() accepts std::optional<void*> - assemble_*_impl.h: Function parameters use std::optional<void*> - assemble_*_impl.h: Kernel calls use .value_or(nullptr) - Python bindings: Handle std::optional with None support - Test: Update to expect None instead of 0 for unset custom_data Benefits: - Clearer intent: "optional value" vs "magic nullptr" - Type safety: Distinguishes "no value" from "null pointer value" - Pythonic: Returns None instead of 0 when not set - Zero-cost: .value_or(nullptr) compiles to same code
cf52114 to
8101685
Compare
jhale
left a comment
There was a problem hiding this comment.
Looks pretty good - a few small changes to make.
I don't think nanobind supports I'm not sure about the argument regarding |
Address review comments to maintain Form immutability: - Remove set_custom_data method from Form class - Pass custom_data as 5th element of integrals tuple at Form creation - Integrals tuple format: (id, kernel_ptr, entities, coeffs, custom_data) - custom_data can be None (maps to std::nullopt) or a pointer (uintptr_t) Pass cell index through entity_local_index for cell integrals: - Cell integrals now receive &cell instead of nullptr for entity_local_index - Enables per-cell data lookup in custom kernels via custom_data - FFCx-generated kernels don't read entity_local_index for cells (return 0) Safety documentation: - Add warning about void pointer safety to Form.h and Python bindings - Document that users must keep data alive while Form is in use Update tests: - Update test_custom_jit_kernels.py to use 5-element tuples - Expand test_custom_data.py with per-cell material property example
| return tabulate | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("dtype", [np.float64]) |
There was a problem hiding this comment.
Would be good to test both np.float64 and np.float32.
| assert L._cpp_object.custom_data(IntegralType.cell, 0, 0) is None | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("dtype", [np.float64]) |
There was a problem hiding this comment.
Extend all tests to use float64 and float32.
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
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