Repository navigation
Retain nonzero columns in unpivoted QR (fixes #28-style column loss) - #1
Merged
Merged
Conversation
The unpivoted QR column skip lived only in `qr_in_place_unblocked`. `qr_in_place_blocked` has no threshold of its own and recurses into that core, so removing the threshold there already covers the wide path; there is no wide-specific branch. Add the transposed fixture as a wide `3 x n` regression test (n = 100/1000/8192). That wide fixture reconstructs to ~4e-16 both with and without the change: at m = 3 the removed threshold `16 (m-k) eps ||column||` is ~1e-14 and never tripped. The tall m = 1000 case goes 2.58e-12 -> 1.98e-15. tlinalg's reported wide `3.33e-7` for this fixture is a reconstruction- indexing bug in that test: it writes entry (i, j) to flat `j + 3i` while the column-major factor uses `i + 3j`. faer's factors for the matrix are accurate to ~4e-16 under the correct indexing.
This was referenced Oct 8, 2026
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
faer's unpivoted Householder QR treats a column as zero when its part orthogonal to the previous columns is below
16*(m-row)*eps*||column||, then skips both the reflector and the row advance, so the column is lost fromQandR. LAPACK has no such skip.Reproduced with the matrix from tensor4all/tlinalg-rs#28,
A = [a, a + 1e-10 e_0, a + 1e-6 e_1]witha = (1,...,1), column-majorm x 3:(
||A - QR|| / ||A||.) The change is the condition: apply the reflector and advance the row whenever the column is not exactly zero, keeping the blocked structure, the trailing update, the tau convention and themin_positive()shortcut. A release smoke showed no measurable cost on100000 x 8(2.984 → 2.986 ms) or1000 x 1000(19.79 → 19.92 ms), 3-run means.cargo test -p t4a-faer: 232 passed, 2 ignored. The fork's ledgerPATCHSET.mdrecords the experiment as kept, with the test and the issues it serves (tlinalg-rs#28,tensor4all-rs#836).