Skip to content

Leave the point indices free in dual evaluation - #288

Merged
pbrubeck merged 3 commits into
mainfrom
pbrubeck/interp-spectral
Aug 26, 2026
Merged

Leave the point indices free in dual evaluation#288
pbrubeck merged 3 commits into
mainfrom
pbrubeck/interp-spectral

Conversation

@pbrubeck

@pbrubeck pbrubeck commented Aug 24, 2026

Copy link
Copy Markdown

Fixes #283 by making dual_evaluation no longer have to sum-factorise internally

dual_evaluation returns (evaluation, point_indices, basis_indices), summing only over the value shape.

TSFC now chooses how to contract over the points, matching basis_evaluation, which already leaves the point index free.

dual_evaluation returns (evaluation, point_indices, basis_indices), summing
only over the value shape. The caller now chooses how to contract over the
points, matching basis_evaluation, which already leaves the point index free.

Dropping the contraction here also retires the workaround for #283.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pbrubeck added a commit to firedrakeproject/firedrake that referenced this pull request Aug 24, 2026
Interpolation compiles through the mode pipeline only once dual evaluation
leaves the point indices free, which lives on the FIAT companion branch. CI
installs the pinned FIAT release, so without this every interpolation raises
"not enough values to unpack (expected 3, got 2)".

Drop this commit once firedrakeproject/fiat#288 merges.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pbrubeck
pbrubeck requested a review from connorjward August 24, 2026 16:05

@connorjward connorjward left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Assuming that tests pass and this doesn't negatively impact performance (by much) then this seems very nice.

@connorjward connorjward left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actually wait, shouldn't we be modifying the stop_at code too?

@pbrubeck

Copy link
Copy Markdown
Author

Actually wait, shouldn't we be modifying the stop_at code too?

Sure, I should do that here.

@pbrubeck
pbrubeck force-pushed the pbrubeck/interp-spectral branch from 8719902 to cec22af Compare August 24, 2026 16:22
@pbrubeck
pbrubeck requested a review from connorjward August 26, 2026 09:32

@connorjward connorjward left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Very nice

@pbrubeck
pbrubeck merged commit 231cc22 into main Aug 26, 2026
9 checks passed
@pbrubeck
pbrubeck deleted the pbrubeck/interp-spectral branch August 26, 2026 11:26
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.

Enhance sum-factorization

2 participants