Repository navigation
Allocate the tabulation buffer in FiniteElement::tabulate - #4656
Open
garth-wells wants to merge 1 commit into
Open
garth-wells wants to merge 1 commit into
garth-wells wants to merge 1 commit into
Conversation
FiniteElement::tabulate forwarded to the Basix overload that allocates and returns a buffer. Allocate the buffer here instead, sizing it with basix::FiniteElement::tabulate_shape and passing it to the overload that takes output storage. The public DOLFINx API and the values returned are unchanged. This is what the Basix overload did internally, so the allocation only moves across the library boundary. Basix removes the allocating overloads in FEniCS/basix#1093. This change compiles against Basix with and without that PR, so it can be merged first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
garth-wells
added a commit
to FEniCS/basix
that referenced
this pull request
Oct 17, 2026
Removing the allocating tabulate overloads breaks DOLFINx main, which calls one of them. Build the DOLFINx integration job against the branch that updates the call site, FEniCS/dolfinx#4656. Revert to main once that pull request has merged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fem::FiniteElement::tabulateforwarded to the Basix overload that allocates and returns a buffer. It now allocates the buffer itself, sizing it withbasix::FiniteElement::tabulate_shapeand passing it to the Basix overload that takes output storage.The public DOLFINx API and the values returned are unchanged. This is exactly what the Basix overload did internally, so the allocation only moves across the library boundary.
Why
Basix removes its allocating
tabulateoverloads in FEniCS/basix#1093, which currently fails its DOLFINx integration check withThis is the only call into a removed overload in DOLFINx.
CoordinateElement.cppalready uses the out-argument overloads, andcpp/demo/custom_kernel/main.cppalready usestabulate_shape.Testing
tabulate_shapeand the out-argument overload exist in Basix with and without FEniCS/basix#1093, so this compiles either way and can be merged before it. Verified by compiling all 45 translation units of the DOLFINx library against both: Basixmainand theremove-allocating-tabulatebranch. Without this change,fem/FiniteElement.cppfails against that branch and builds againstmain.🤖 Generated with Claude Code