Uh oh!
There was an error while loading. Please reload this page.
This repository was archived by the owner on Aug 20, 2026. It is now read-only.
perf(reg_flow): route bending==0 Lamé calls to matvec_lame/diag_lame - #87
Merged
Conversation
`_flow_matvec`, `_flow_matvec_acc` and `_flow_diag` sent *any* non-zero `shears`/`div` straight to the full combined `matvec_all`/`diag_all` stencil with no `bending != 0` guard, leaving the cheaper Lame-only `matvec_lame` / `diag_lame` stencils unreachable from the public dispatch even though they exist in the impl and are already used by `relax_lame_`. `_flow_kernel` and `_flow_relax` in this same file already nest the branch correctly; this mirrors that exact nesting into the remaining three. Numerically a no-op. With bending == 0, make_kernel_all's bending-only taps (w200, w020, w110 in 2D) are all exactly zero and its remaining entries equal make_kernel_lame's one-for-one, so `all` is `lame` plus zero-weighted taps -- identical results for finite inputs, just more work per voxel. Elastic-only (non-bending) registration is a common configuration and now pays roughly a 9-tap stencil per voxel per channel instead of ~25. The RLS/JRLS dispatches are deliberately untouched: only `*_lame_jrls` and `*_membrane_jrls` exist at the impl layer (there is no bending-aware JRLS kernel), and bending is rejected by the public wrapper before dispatch, so they already call the Lame kernel unconditionally. The `is_matrix` uses in the `flow_kernel` wrappers select the output tensor's rank, not a stencil, and are likewise unaffected by bending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Submodule staleness (non-blocking -- fastfields-lib#15)
|
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 freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Re: #51 (already closed — see "Issue status" below).
The defect
_flow_matvec,_flow_matvec_accand_flow_diaginreg_flow.cppsent any non-zeroshears/divstraight to the full combinedmatvec_all/diag_allstencil, with nobending != 0guard:That leaves the cheaper Lamé-only
matvec_lame/diag_lamestencils unreachable from the public dispatch, even though they exist in the impl and are already exercised viarelax_lame_._flow_kerneland_flow_relaxin this same file already nest the branch correctly; this mirrors that exact nesting into the remaining three.Sites
_flow_matvec_flow_matvec_acc_flow_diag_flow_kernel,_flow_relax_flow_matvec_rls,_flow_diag_rls,_flow_relax_rls*_lame_jrls/*_membrane_jrlsexist at the impl layer (there is no bending-aware JRLS kernel), and bending is rejected by the public wrapper before dispatch (reg_flow.cpp:1254), so these already call the Lamé kernel unconditionallyis_matrixuses in theflow_kernelwrappersWhy this cannot change results
Not just asserted from the test suite — checked in the kernel builders. With
bending == 0,make_kernel_all's bending-only taps are all exactly zero:and the surviving entries equal
make_kernel_lame's one-for-one (kernel[0..2]/[6..8]/[12]ofallmap onto[0..2]/[3..5]/[6]oflame). Soallatbending == 0islameplus zero-weighted taps — adding0.0 * xto a running sum is exact for finitex, so results are bit-identical, and the only difference is work done. Roughly a 9-tap stencil per voxel per channel instead of ~25 for elastic-only (non-bending) registration, a common configuration.Verification
make -j2 test CXX=clang++, before and after this change, with everything else held fixed (same kernels commit, same impl):The full suite report is byte-identical —
diffof the two runs'running/checks:/PASSEDlines is empty, 37 lines each, 13/13 suites, 0 failures:Coverage of the newly-reachable path is real rather than incidental —
test_reg_flowalready runsbending == 0, shears/div != 0cases through the Lamé symmetry and diag tests, andrun_2d_kernel_impulsewithbending = 0cross-checksflow_matvecagainst the materialisedflow_kernelstencil. Since_flow_kernelwas already onkernel_lameforbending == 0, that test previously comparedmatvec_allagainstkernel_lameand now comparesmatvec_lameagainstkernel_lame— independent confirmation the two agree.Issue status
#51 describes this defect but is already closed (2026-08-05), auto-closed by the unrelated merged PR #82 which referenced it. The defect was nevertheless still live on
main— I re-verified the three unguarded dispatch sites directly before changing anything. So this PR deliberately does not say "Closes#51"; someone may want to confirm #51's closure was intended.Provenance
The same change exists on the
teenybranch in8afcab4.fastfields-cpu-lib#52was an attempt to port it tomain, but it was filed from theclaude-fastfields-to-teenyworkstream, whose PRs must always targetteeny— so it was mis-filed, not wrong. Its analysis is sound and was useful prior art; credit to it. This PR lands the equivalent change onmainproperly, re-derived and re-verified against currentmain(where_flow_diagis now templated onchar op, and_flow_matvec_accdid not exist when the original was written).🤖 Generated with Claude Code
https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Generated by Claude Code