test(linalg): add contract tests for flux-independent QR/LQ isometry - #460
Conversation
Add qr_orthogonality_holds_for_nonzero_flux and lq_orthogonality_holds_for_nonzero_flux, which pin block-sparse isometry as a per-sector orthogonality condition independent of the tensor's overall flux label, using a non-identity flux fixture. Extract the shared Q^T Q = I / Q Q^T = I checks into assert_orthonormal_columns / assert_orthonormal_rows helpers and route the existing identity-flux orthogonality tests through them as well.
📝 WalkthroughWalkthroughQR and LQ orthogonality assertions now use shared per-sector isometry helpers, with additional tests covering nonzero-flux tensor fixtures. ChangesFlux-independent isometry tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR strengthens the block-sparse decomposition test suite by making “flux-independent per-sector isometry” an explicit, named contract for QR/LQ factors in ariadnetor-linalg.
Changes:
- Adds
qr_orthogonality_holds_for_nonzero_fluxandlq_orthogonality_holds_for_nonzero_fluxusing the existingsample_u1_nonzero_flux()fixture. - Extracts shared orthogonality checks into
assert_orthonormal_columns/assert_orthonormal_rowsand reuses them from the existing identity-flux orthogonality tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ariadnetor-linalg/src/block_sparse_decomp/tests/mod.rs`:
- Around line 369-425: Extract the duplicated Gram-matrix accumulation and
identity assertion logic from assert_orthonormal_columns and
assert_orthonormal_rows into a shared test helper. Parameterize the helper as
needed for the row/column multiplication orientation and dimensions, then have
both existing functions delegate to it while preserving their current tolerances
and behavior.
- Around line 369-425: Remove the redundant PartialEq bound from the generic
signatures of assert_orthonormal_columns and assert_orthonormal_rows. Keep S
constrained only by Sector, leaving their existing calls to
compute_fused_sector_groups and assemble_sector_matrix unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0e710a6c-2f05-4050-a5a5-73f8cbbad874
📒 Files selected for processing (1)
crates/ariadnetor-linalg/src/block_sparse_decomp/tests/mod.rs
Summary
Add contract tests that name flux-independent isometry as the invariant under test for block-sparse QR/LQ. The property — per-sector orthogonality holds regardless of the tensor's overall flux label — was already implicitly exercised by the identity-flux
qr_orthogonality/lq_orthogonalitytests, but no test name or docstring called it out. Making it an explicitly named contract, pinned by a non-identity flux fixture, keeps a future reader from conflating the overall flux label with the per-sector isometry condition.Closes #115
Changes
qr_orthogonality_holds_for_nonzero_fluxandlq_orthogonality_holds_for_nonzero_flux, driven by the existingsample_u1_nonzero_flux()fixture (flux =U1Sector(1)), assertingassert_orthonormal_columns/assert_orthonormal_rowshelpers and route the existing identity-flux orthogonality tests through them, collapsing four near-identical loop bodies into two helpers.Test plan
cargo test -p ariadnetor-linalg: 300 passed, 0 failed (the four orthogonality tests, two new and two refactored, all green).@coderabbitai ignore