fix(householder): split multiple-block factor upgrades at a block boundary (#265) by shinaoka · Pull Request #343 · tensor4all/faer

shinaoka · 2026-11-09T09:30:55Z

Fixes the multiple-block branch of upgrade_householder_factor, which split the square factor at a number of blocks where it needed a column offset.

Cause

faer/src/linalg/householder.rs:198 (main f3873492) computed mid = block_count / 2 and passed mid to split_at_mut(mid, mid), split_at_col(mid) and split_at_row(mid) (l.199-201). Those take a row/column offset. So once block_count >= 2, a half could start inside a target block. Pairs that straddled the split were never written, and the second half's block grid was shifted, so it wrote coefficients between blocks. In the issue's case (n = 9, block_size = 2, prev_block_size = 1) the recursion splits at global columns 2, 4, 5, 6 and 7 (each half re-counting its own blocks from its first column). Pairs (4,5) and (6,7) straddle a split, so they are never formed, and columns {7,8} become a leaf of their own, which writes (7,8). Those are exactly the three entries the issue reports.

The previous-block branch of the same function already splits on a boundary: mid = (prev_block_count / 2) * prev_block_size (l.343). The fix uses that rule for target blocks: mid = (block_count / 2) * block_size. The two halves still go to join_raw with the team split it makes; no width or floor changes.

Scratch side: on main this function has no scratch query (its row-split partials are allocated internally, see #93), and no *_scratch function mirrors this recursion, so there is no second partition to align. The scratch API from the closed #267 is not part of this PR.

Callers: every in-tree caller (blocked QR, col-pivoting QR, Hessenberg, tridiagonal, bidiagonal) passes a factor exactly block_size wide. They only reach the single-block branch, so their results are not expected to change.

The same mid = block_count / 2 is in upstream Codeberg 0ecd846 (householder.rs:158). I found this by reading the source; I did not run it there. It is recorded in the worklog under the deferred-feedback rule, and nothing was sent upstream.

Changes

faer/src/linalg/householder.rs: the one-line split fix plus a comment.
faer/tests/public_householder_upgrade.rs (new), a public-API test:
- issue_265_reproduction: the issue's MWE as filed (f64, Seq).
- several_blocks_{f32,f64,c32,c64}: run on Par::Seq. Cases are the issue's shape (4 blocks + 1-column tail), square and tall; two full blocks with no tail; 3 blocks (4,4,3) over prev = 2; 5 blocks (8x4+5) over prev = 4; 5 blocks (16x4+6) over prev = 8, which takes the branch that keeps the previous blocks' coefficients. Controls that pass before and after: prev == block_size (a no-op) and single blocks where prev = 4 does not divide block_size = 6.
- several_blocks_on_a_team: 4096-row cases (n = 9, 11, 16, 37, 70) at all four scalars on 2- and 4-thread teams. The n = 37 and n = 70 cases must open team regions (checked with faer::utils::team::stats).
- several_blocks_reject_a_previous_size_that_does_not_divide: with several blocks and prev not dividing block_size, the call still panics. The test pins that.
- Every entry is checked on its own. Each within-block coefficient is compared with V^H V formed in c64, within 4 m eps sum_k |V_ki||V_kj|. Diagonal (denominators), lower triangle and between-block entries must stay exactly equal to a sentinel/tau. The basis's diagonal and upper part hold garbage, so any read of them would fail the test.
.github/workflows/ci.yml: adds --test public_householder_upgrade to the std,linalg job. The team test is behind cfg(feature = "threadteam"), so that job runs the other six.
PATCHSET.md row and docs/worklogs/2026-11-09-householder-upgrade-block-offset.md.

Verification

Host: aarch64 macOS (Apple Silicon), toolchain 1.96.0 (pinned).

Red: the new test at head 520f6c5d with householder.rs reverted to main f3873492 (fix not applied). Each test reports every failing shape, so this one run shows each case on its own. The issue_265_reproduction output is the same three entries the issue reports.

$ cargo test -j 8 -p t4a-faer --test public_householder_upgrade
test issue_265_reproduction ... FAILED
test several_blocks_reject_a_previous_size_that_does_not_divide - should panic ... ok
test several_blocks_c32 ... FAILED
test several_blocks_f32 ... FAILED
test several_blocks_f64 ... FAILED
test several_blocks_c64 ... FAILED
test several_blocks_on_a_team ... FAILED
---- issue_265_reproduction ----
independent block Gram coefficients differ:
(4,5): expected 0.13, got 0
(6,7): expected 0.11, got 0
(7,8): expected 0, got 0.1
---- several_blocks_{c32,f32,f64,c64} ----
7 failing shapes:
m=9 n=9 block=2 prev=1 [num_complex::Complex<f32>]: 3 wrong entries:
m=40 n=9 block=2 prev=1 [num_complex::Complex<f32>]: 3 wrong entries:
m=20 n=16 block=8 prev=1 [num_complex::Complex<f32>]: 28 wrong entries:
m=11 n=11 block=4 prev=2 [num_complex::Complex<f32>]: 11 wrong entries:
m=30 n=11 block=4 prev=2 [num_complex::Complex<f32>]: 11 wrong entries:
m=60 n=37 block=8 prev=4 [num_complex::Complex<f32>]: 79 wrong entries:
m=90 n=70 block=16 prev=8 [num_complex::Complex<f32>]: 300 wrong entries:
7 failing shapes:
m=9 n=9 block=2 prev=1 [f32]: 3 wrong entries:
m=40 n=9 block=2 prev=1 [f32]: 3 wrong entries:
m=20 n=16 block=8 prev=1 [f32]: 28 wrong entries:
m=11 n=11 block=4 prev=2 [f32]: 11 wrong entries:
m=30 n=11 block=4 prev=2 [f32]: 11 wrong entries:
m=60 n=37 block=8 prev=4 [f32]: 79 wrong entries:
m=90 n=70 block=16 prev=8 [f32]: 300 wrong entries:
7 failing shapes:
m=9 n=9 block=2 prev=1 [f64]: 3 wrong entries:
m=40 n=9 block=2 prev=1 [f64]: 3 wrong entries:
m=20 n=16 block=8 prev=1 [f64]: 28 wrong entries:
m=11 n=11 block=4 prev=2 [f64]: 11 wrong entries:
m=30 n=11 block=4 prev=2 [f64]: 11 wrong entries:
m=60 n=37 block=8 prev=4 [f64]: 79 wrong entries:
m=90 n=70 block=16 prev=8 [f64]: 300 wrong entries:
7 failing shapes:
m=9 n=9 block=2 prev=1 [num_complex::Complex<f64>]: 3 wrong entries:
m=40 n=9 block=2 prev=1 [num_complex::Complex<f64>]: 3 wrong entries:
m=20 n=16 block=8 prev=1 [num_complex::Complex<f64>]: 28 wrong entries:
m=11 n=11 block=4 prev=2 [num_complex::Complex<f64>]: 11 wrong entries:
m=30 n=11 block=4 prev=2 [num_complex::Complex<f64>]: 11 wrong entries:
m=60 n=37 block=8 prev=4 [num_complex::Complex<f64>]: 79 wrong entries:
m=90 n=70 block=16 prev=8 [num_complex::Complex<f64>]: 300 wrong entries:
40 failing shapes:
width=2 m=4096 n=9 block=2 prev=1 [f32]: 3 wrong entries:
width=2 m=4096 n=11 block=4 prev=2 [f32]: 10 wrong entries:
width=2 m=4096 n=16 block=8 prev=1 [f32]: 27 wrong entries:
width=2 m=4096 n=37 block=8 prev=4 [f32]: 77 wrong entries:
width=2 m=4096 n=70 block=16 prev=8 [f32]: 284 wrong entries:
width=2 m=4096 n=9 block=2 prev=1 [f64]: 3 wrong entries:
width=2 m=4096 n=11 block=4 prev=2 [f64]: 11 wrong entries:
width=2 m=4096 n=16 block=8 prev=1 [f64]: 28 wrong entries:
width=2 m=4096 n=37 block=8 prev=4 [f64]: 79 wrong entries:
width=2 m=4096 n=70 block=16 prev=8 [f64]: 300 wrong entries:
width=2 m=4096 n=9 block=2 prev=1 [num_complex::Complex<f32>]: 3 wrong entries:
width=2 m=4096 n=11 block=4 prev=2 [num_complex::Complex<f32>]: 11 wrong entries:
width=2 m=4096 n=16 block=8 prev=1 [num_complex::Complex<f32>]: 28 wrong entries:
width=2 m=4096 n=37 block=8 prev=4 [num_complex::Complex<f32>]: 79 wrong entries:
width=2 m=4096 n=70 block=16 prev=8 [num_complex::Complex<f32>]: 298 wrong entries:
width=2 m=4096 n=9 block=2 prev=1 [num_complex::Complex<f64>]: 3 wrong entries:
width=2 m=4096 n=11 block=4 prev=2 [num_complex::Complex<f64>]: 11 wrong entries:
width=2 m=4096 n=16 block=8 prev=1 [num_complex::Complex<f64>]: 28 wrong entries:
width=2 m=4096 n=37 block=8 prev=4 [num_complex::Complex<f64>]: 79 wrong entries:
width=2 m=4096 n=70 block=16 prev=8 [num_complex::Complex<f64>]: 300 wrong entries:
width=4 m=4096 n=9 block=2 prev=1 [f32]: 3 wrong entries:
width=4 m=4096 n=11 block=4 prev=2 [f32]: 10 wrong entries:
width=4 m=4096 n=16 block=8 prev=1 [f32]: 27 wrong entries:
width=4 m=4096 n=37 block=8 prev=4 [f32]: 77 wrong entries:
width=4 m=4096 n=70 block=16 prev=8 [f32]: 284 wrong entries:
width=4 m=4096 n=9 block=2 prev=1 [f64]: 3 wrong entries:
width=4 m=4096 n=11 block=4 prev=2 [f64]: 11 wrong entries:
width=4 m=4096 n=16 block=8 prev=1 [f64]: 28 wrong entries:
width=4 m=4096 n=37 block=8 prev=4 [f64]: 79 wrong entries:
width=4 m=4096 n=70 block=16 prev=8 [f64]: 300 wrong entries:
width=4 m=4096 n=9 block=2 prev=1 [num_complex::Complex<f32>]: 3 wrong entries:
width=4 m=4096 n=11 block=4 prev=2 [num_complex::Complex<f32>]: 11 wrong entries:
width=4 m=4096 n=16 block=8 prev=1 [num_complex::Complex<f32>]: 28 wrong entries:
width=4 m=4096 n=37 block=8 prev=4 [num_complex::Complex<f32>]: 79 wrong entries:
width=4 m=4096 n=70 block=16 prev=8 [num_complex::Complex<f32>]: 298 wrong entries:
width=4 m=4096 n=9 block=2 prev=1 [num_complex::Complex<f64>]: 3 wrong entries:
width=4 m=4096 n=11 block=4 prev=2 [num_complex::Complex<f64>]: 11 wrong entries:
width=4 m=4096 n=16 block=8 prev=1 [num_complex::Complex<f64>]: 28 wrong entries:
width=4 m=4096 n=37 block=8 prev=4 [num_complex::Complex<f64>]: 79 wrong entries:
width=4 m=4096 n=70 block=16 prev=8 [num_complex::Complex<f64>]: 300 wrong entries:
test result: FAILED. 1 passed; 6 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.41s

All 7 multiple-block shapes fail at all four scalars on Par::Seq, and all 40 team runs fail (5 shapes x 4 scalars x widths 2 and 4). The three controls (prev == block_size and the two single-block shapes) are not listed because they pass with or without the fix.

Green at 520f6c5d:

$ cargo test -j 8 -p t4a-faer --test public_householder_upgrade
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
$ cargo test -j 8 -p t4a-faer --no-default-features --features "std,linalg" --test public_householder_upgrade
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
$ cargo test -j 8 -p t4a-faer --lib
test result: ok. 691 passed; 0 failed; 162 ignored; 0 measured; 0 filtered out; finished in 18.65s
$ cargo test --no-fail-fast -j 8 -p t4a-faer --test lq --test qr_rank_truncation --test public_bidiag_sequential --test public_bidiagonal_svd --test public_hessenberg_schur --test self_adjoint_generalized_eigen --test svd_values_no_alloc --test evd_svd_vectors_no_alloc
lq 3 passed; qr_rank_truncation 5 passed; public_bidiag_sequential 2 passed; public_bidiagonal_svd 6 passed;
public_hessenberg_schur 2 passed; self_adjoint_generalized_eigen 9 passed; svd_values_no_alloc 8 passed;
evd_svd_vectors_no_alloc 0 passed 1 ignored

Two failures in the same run that main has too:

lq_no_alloc::lq_allocates_nothing_60x300 fails with [13, 12, 17, 17] allocations. Unmodified main gives the same counts on this host. This is the aarch64 SME pack-buffer case described in faer/tests/lq_no_alloc.rs:17-29.
no_step_alloc::parallel_regions_allocate_nothing failed once in the combined run (full pivoting LU, width 4: 2). LU does not call this function. It then passed 5/5 alone with the fix and 5/5 on main.

cargo fmt --check reports nothing for the changed lines or the new file. The repo-wide fmt and clippy jobs are continue-on-error and already report many hits on main.

Not measured: timing. The fix changes which coefficients are formed (at n = 9, before: (0,1), (2,3), (7,8); after: (0,1), (2,3), (4,5), (6,7)). No dispatch or floor code was edited.

The related integration-test run was at b3957170. The only later change is in the new test file, so householder.rs is identical there.

Closes #265

🤖 Generated with Claude Code

…ndary upgrade_householder_factor's multiple-block branch split the factor and the basis at `block_count / 2`, a block count passed to row/column splits, so a half could start inside a target block. At n = 9, block_size = 2, prev_block_size = 1 it left T[4, 5] and T[6, 7] unwritten and wrote T[7, 8] between blocks. Split at `(block_count / 2) * block_size`, the rule the previous-block branch already uses; the join_raw team split is unchanged. Adds faer/tests/public_householder_upgrade.rs (f32/f64/c32/c64, Seq and 2/4-thread teams, odd block counts, short last blocks, previous widths 1/2/4/8): each within-block coefficient, the denominators and every entry outside the blocks are checked separately. Registers it in the std,linalg CI job. Records the fix in PATCHSET.md and a worklog. Closes #265 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

shinaoka · 2026-11-09T09:45:50Z

GPT-6.1-sol adversarial review

CRITICAL: none
NEEDS_CHANGES: 1
UNVERIFIED: 1

Critical (must not merge)

none

Needs changes

PATCHSET.md:120 — “the same products over the same total work” contradicts the recursion. Reproduce with (m, n, block_size, prev_block_size) = (9, 9, 2, 1) and enumerate the strict-upper coefficients formed by each leaf: before the fix they are (0,1), (2,3), (7,8); afterward they are (0,1), (2,3), (4,5), (6,7). The fix therefore changes both the products and their count, rather than merely repartitioning identical work. Remove that assertion from the ledger and PR body; state that timing was not measured and the dispatch-policy code was not edited.

Unverified claims

docs/worklogs/2026-11-09-householder-upgrade-block-offset.md:26-30 and the PR’s Verification section — the complete before/after matrix and broader regression counts remain unverified. Reproduce the evidence gap by reverting the split and running the supplied test binary: seq_all stops at its first failing shape (faer/tests/public_householder_upgrade.rs:212-215), and the team test likewise stops at its first failure, so that output does not establish that every listed shape fails independently. I reproduced six failing tests with the original split, but cannot read the historical per-shape runs, 691-library-test run, or baseline allocation comparisons from the supplied artifacts. The exact-head hosted build and testing-stable-ubuntu-latest jobs were still in progress when inspected; their results do not yet supply the missing verification.

Checked and fine

faer/tests/public_householder_upgrade.rs: direct rustc --test execution passed all seven tests; rebuilding with the original split produced one pass and six failures.
Full Cargo test execution was not completed: attempts timed out during vendored OpenSSL compilation.
kache doctor --repair --verify completed with 64,469 valid entries and zero corrupted entries.

Reviewed head: b3957170b9f3586c85327b35ab41e08adef36b93 at 2026-11-09T09:31:06Z. This review is valid only for that commit: if the head has moved, it is a review of a tree that is not being merged.
Automated (gpt_review.sh). This is an enumeration of counterexamples, not a verdict; the merge decision stays with a human.

…dger's work claim The sequential and team tests now collect the failures of all shapes before asserting, so one run shows each shape fails or passes on its own instead of stopping at the first. PATCHSET.md no longer says the fix keeps the same products: before it the n = 9 leaves formed (0,1), (2,3), (7,8), and now (0,1), (2,3), (4,5), (6,7). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

shinaoka · 2026-11-09T09:58:19Z

GPT-6.1-sol adversarial review

CRITICAL: none
NEEDS_CHANGES: 0
UNVERIFIED: 2

Critical (must not merge)

none

Needs changes

none

Unverified claims

Broader regression and allocation evidence (docs/worklogs/2026-11-09-householder-upgrade-block-offset.md:34–37; PR body, “Verification”): I did not reproduce the claimed 691 library passes, related integration-test counts, identical LQ allocation counts on main, or the 5/5 allocation-test reruns. The diff cannot establish those results. Reproduce with the listed commands at the stated revisions, comparing allocation failures against unmodified main; the focused regression results below do not establish those broader claims.
Completed CI validation of this head (.github/workflows/ci.yml:30, .github/workflows/run-tests.yml:124): At inspection, both build and testing-stable-ubuntu-latest were still in_progress for 520f6c5d154599622dc781514d7836f7b9d07a3b. The completed description/MSRV/no-std checks therefore do not establish that the regression tests passed in CI. Recheck with gh pr checks 343 --repo tensor4all/faer; the sequential-only command excludes the team test, while the default-feature coverage job includes it.

Checked and fine

Exact-head focused tests: default features 7 passed; --no-default-features --features std,linalg 6 passed.
Replacing only the corrected offset with block_count / 2 in an isolated copy produced 6 failing tests, including all 40 team cases and the three reported incorrect entries.
Not checked: full suites, allocation baselines, or timing. Cache repair/verification completed with 0 corrupted entries; pending CI remains unverified.

Reviewed head: 520f6c5d154599622dc781514d7836f7b9d07a3b at 2026-11-09T09:48:26Z. This review is valid only for that commit: if the head has moved, it is a review of a tree that is not being merged.
Automated (gpt_review.sh). This is an enumeration of counterexamples, not a verdict; the merge decision stays with a human.

shinaoka requested a review from terasakisatoshi as a code owner November 9, 2026 09:30

shinaoka merged commit 4ccd69b into main Nov 9, 2026
9 checks passed

shinaoka deleted the fix/265-householder-block-offset branch November 9, 2026 10:18

Navigation Menu

Repository navigation

fix(householder): split multiple-block factor upgrades at a block boundary (#265) - #343

fix(householder): split multiple-block factor upgrades at a block boundary (#265)#343
shinaoka merged 2 commits into
mainfrom
fix/265-householder-block-offset

shinaoka commented Nov 9, 2026 •

edited

Loading

Uh oh!

shinaoka commented Nov 9, 2026

Uh oh!

shinaoka commented Nov 9, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

shinaoka commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cause

Changes

Verification

Uh oh!

shinaoka commented Nov 9, 2026

GPT-6.1-sol adversarial review

Critical (must not merge)

Needs changes

Unverified claims

Checked and fine

Uh oh!

shinaoka commented Nov 9, 2026

GPT-6.1-sol adversarial review

Critical (must not merge)

Needs changes

Unverified claims

Checked and fine

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

shinaoka commented Nov 9, 2026 •

edited

Loading