Skip to content
This repository was archived by the owner on Aug 20, 2026. It is now read-only.

test(reg): cover the diag_bending/diag_all corner cross-term bug - #89

Merged
balbasty merged 1 commit into
mainfrom
claude/48-diag-boundary-symmetry
Aug 18, 2026
Merged

test(reg): cover the diag_bending/diag_all corner cross-term bug#89
balbasty merged 1 commit into
mainfrom
claude/48-diag-boundary-symmetry

Conversation

@balbasty

Copy link
Copy Markdown
Contributor

Regression test for the defect fixed in fastfields-kernels#81 (issue fastfields-kernels#48). Test-only: +230 lines across the two test drivers, no production code touched.

The oracle — no reference implementation needed

The boundary-corrected diagonal expanded each corner weight as (fx0*fy0 + fx1*fy0 + fx1*fy0 + fx1*fy1), double-counting fx1*fy0 and dropping fx0*fy1.

On a square/cubic domain with the same boundary condition and voxel size on every axis, relabelling two spatial axes maps the operator onto itself, so its diagonal must be invariant under that relabelling:

caseinvariant
field, 2D and 3Ddiag(x,y,c) == diag(y,x,c)
flow, bending-only (shears == div == 0)diag(x,y,c) == diag(y,x,c) — channels uncoupled, isotropic voxel size
flow, diag_alldiag(x,y,c) == diag(y,x,1-c) — the Lamé terms couple the channels, so the axis swap carries a channel swap; shears == div keeps the Lamé part symmetric under that simultaneous relabelling

The buggy corner sum is not symmetric under the fx <-> fy relabelling wherever the two axes' one-sided boundary signs differ. It cancels exactly where they agree (fx0==fx1 and fy0==fy1 make both forms 4*fx0*fy0) — which is precisely why the pre-existing interior-only diagonal checks never caught it.

What this covers beyond the minimum

  • 3D as well as 2D. The 3D diagonal carries three independent corner cross-terms (w110/w101/w011) rather than one; the xy and yz swaps exercise two directly and the third by composition. 9 of the 12 fixed terms live in 3D, so 2D-only coverage would leave most of the fix untested.
  • Whole-domain sweep rather than a single row/column.
  • Three boundary conditions — Zero, DCT2, DST2. DCT2 is a deliberate control: it does not flip signs, so it passes with or without the fix.

Verification — it demonstrably bites

Same tests, same build flags, same binaries; the only thing changed between the two runs is the kernels headers:

kernelstest_reg_fieldtest_reg_flow
pre-fix (buggy corner term)19250 checks, 328 failures → FAILED (exit 1)16347 checks, 1696 failures → FAILED (exit 1)
1df9fd3 (the fix)19250 checks, 0 failures → PASSED16347 checks, 0 failures → PASSED

Check counts are identical in both directions, so the only difference is pass/fail. Every one of the 2024 failures is in the new checks — no pre-existing test changes value:

 68 field2d_diag_bending.axis_symmetry
130 field3d_diag_bending.axis_symmetry_xy
130 field3d_diag_bending.axis_symmetry_yz
68 flow2d_diag_bending.axis_symmetry
68 flow2d_diag_all.axis_symmetry
390 flow3d_diag_bending.axis_symmetry_xy 390 flow3d_diag_bending.axis_symmetry_yz
390 flow3d_diag_all.axis_symmetry_xy 390 flow3d_diag_all.axis_symmetry_yz

Discrepancies are large and unambiguous (e.g. got 17 expected 21, got 32.3 expected 36.3) against a 1e-5 tolerance — not a marginal-tolerance test.

Ordering

This depends on the pin chain already reaching the fix, which it now does on main:

cpu-lib bdbe80d -> cpu-impl 5e2c78e -> kernels 1df9fd3

(kernels#81 → cpu-impl#72 → cpu-lib#88, all merged.)

Provenance

An equivalent 2D-only test exists on the teeny branch in 8afcab4. fastfields-cpu-lib#53 was an attempt to port it to main, but it was filed from the claude-fastfields-to-teeny workstream, whose PRs must always target teeny — so it was mis-filed, not wrong. Its oracle is sound and was useful prior art; credit to it. This PR lands equivalent (and broader — 3D, whole-domain, multi-bound) coverage on main properly, re-derived and re-verified from scratch.

🤖 Generated with Claude Code

https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z


Generated by Claude Code

Regression test for fastfields-kernels#48 / #81: the boundary-corrected
diagonal expanded each corner weight as
(fx0*fy0 + fx1*fy0 + fx1*fy0 + fx1*fy1), double-counting fx1*fy0 and
dropping fx0*fy1.
The oracle needs no reference implementation. On a square/cubic domain with
the same boundary condition and voxel size on every axis, relabelling two
spatial axes maps the operator onto itself, so its diagonal must be invariant
under that relabelling:
* field, 2D and 3D: diag(x,y,c) == diag(y,x,c)
* flow bending-only: diag(x,y,c) == diag(y,x,c) (channels uncoupled)
* flow diag_all: diag(x,y,c) == diag(y,x,1-c) (the Lame terms couple
the channels, so the axis swap carries a channel
swap; shears == div keeps the Lame part symmetric
under that simultaneous relabelling)
The 3D cases matter on their own: the 3D diagonal carries three independent
corner cross-terms (w110/w101/w011) rather than one, so the xy and yz swaps
exercise two directly and the third by composition.
Each test sweeps the whole domain rather than a single row/column, and runs
under Zero, DCT2 and DST2. DCT2 is a deliberate control: it does not flip
signs, so it passed even before the fix -- the error only appears where the
two axes' one-sided boundary signs differ.
Verified to bite. Same tests, same binaries, only the kernels headers
differing:
kernels test_reg_field test_reg_flow
pre-fix 19250 checks / 328 failures 16347 checks / 1696 failures
1df9fd3 19250 checks / 0 failures 16347 checks / 0 failures
Check counts are identical in both directions, so the only difference is
pass/fail, and every failure is in the new checks -- no pre-existing test
changes value.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
@github-actions

Copy link
Copy Markdown

Submodule staleness (non-blocking -- fastfields-lib#15)

submodulepinned committracksstatusbehind by
impl5e2c78efastfields/fastfields-cpu-impl@mainup to date0

@balbasty
balbasty merged commit 1fb2b37 into mainAug 18, 2026
6 of 7 checks passed
@balbasty
balbasty deleted the claude/48-diag-boundary-symmetry branch August 18, 2026 15:05
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude