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

fix(regularisers): correct diag_bending corner cross-term sign error - #32

Merged
balbasty merged 1 commit into
claude/fastfields-teeny-refactor-js42idfrom
claude/regularisers-diag-bugfix
Aug 1, 2026
Merged

fix(regularisers): correct diag_bending corner cross-term sign error#32
balbasty merged 1 commit into
claude/fastfields-teeny-refactor-js42idfrom
claude/regularisers-diag-bugfix

Conversation

@balbasty

Copy link
Copy Markdown
Collaborator

Summary

  • The boundary-corrected diagonal for diag_bending (field) and diag_bending/diag_all (flow) computed the corner weight as w*(fx0*fy0 + fx1*fy0 + fx1*fy0 + fx1*fy1)fx1*fy0 counted twice, fx0*fy1 dropped entirely.
  • The correct expansion of (fx0+fx1)*(fy0+fy1) is fx0*fy0 + fx1*fy0 + fx0*fy1 + fx1*fy1, matching what matvec_bending/matvec_all already compute.
  • Fixes 6 functions across regularisers/field/{2,3}d.h and regularisers/flow/{2,3}d.h (12 wrong terms total).
  • The bug cancels exactly at fully-symmetric corners (both axes flip the same way) and only manifests where the two axes' one-sided boundary signs differ, so it was invisible to the existing interior-only diag tests. It corrupts the shipped field_diag/flow_diag public API output (the preconditioner diagonal exposed to numpy/torch/cupy) at boundary voxels under sign-flipping bounds (DST1/DST2/Zero) whenever bending (or flow "all") is active.

Test plan

  • New regression tests added downstream in fastfields-cpu-lib (test_reg_field.cpp, test_reg_flow.cpp), asserting diag(0,j,c) == diag(j,0,c) on a square domain with a uniform sign-flipping boundary condition (DST2) — a property that must hold regardless of the exact stencil.
  • Confirmed via git stash/rebuild that these tests fail against the pre-fix code and pass against the fix.
  • Full CPU suite (make test CXX=clang++ in fastfields-cpu-lib) green: 0 failures across all 11 test binaries.

Generated by Claude Code

The boundary-corrected diagonal for diag_bending (field) and
diag_bending/diag_all (flow) computed the corner weight as
w*(fx0*fy0 + fx1*fy0 + fx1*fy0 + fx1*fy1) -- fx1*fy0 counted twice and
fx0*fy1 dropped. The correct expansion of (fx0+fx1)*(fy0+fy1) is
fx0*fy0 + fx1*fy0 + fx0*fy1 + fx1*fy1, matching what matvec_bending/
matvec_all already compute. The bug cancels at fully-symmetric corners
(both axes flip) and only manifests where the two axes' one-sided
boundary signs differ, so it was invisible to the existing interior-only
diag tests.
Fixes 6 functions across field/{2,3}d.h and flow/{2,3}d.h (12 wrong
terms total).
@balbastyClaude

Copy link
Copy Markdown
CollaboratorAuthor

Agent:claude-fastfields-to-teeny — opened from the fastfields teeny-refactor session (claude/fastfields-teeny-refactor-js42id). Flagging for disambiguation from the concurrent claude-jitfields-to-fastfields agent (claude/jitfields-fastfields-migration-v5r416), which is porting jitfields to fastfields directly, without teeny.


Generated by Claude Code

@balbasty
balbasty merged commit 5ed785a into claude/fastfields-teeny-refactor-js42idAug 1, 2026
1 check failed
@balbasty
balbasty deleted the claude/regularisers-diag-bugfix branch August 1, 2026 08:17
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude