Skip to content

fix(matrix): reject overflowed symmetry tolerance scaling#128

Merged
acgetchell merged 1 commit into
mainfrom
fix/94-tolerance-invariants
Jun 3, 2026
Merged

fix(matrix): reject overflowed symmetry tolerance scaling#128
acgetchell merged 1 commit into
mainfrom
fix/94-tolerance-invariants

Conversation

@acgetchell
Copy link
Copy Markdown
Owner

@acgetchell acgetchell commented Jun 3, 2026

  • Enforce the tolerance contract around symmetry checks by surfacing scaled tolerance overflow as a typed non-finite intermediate error.
  • Document finite, non-negative tolerance requirements across tolerance-taking matrix APIs.
  • Add regression coverage for invalid tolerance construction and symmetry tolerance overflow.
  • Update exact examples to propagate typed crate errors instead of unwrapping.

Closes #94

Summary by CodeRabbit

  • New Features

    • API now enforces stricter validation for numeric operations, returning errors for non-finite values, invalid tolerances, and overflow conditions.
  • Documentation

    • Clarified tolerance construction requirements and numeric error conditions across library functions.
  • Tests

    • Added comprehensive test coverage for tolerance validation and numeric error cases including overflow scenarios.

- Enforce the tolerance contract around symmetry checks by surfacing scaled
  tolerance overflow as a typed non-finite intermediate error.
- Document finite, non-negative tolerance requirements across tolerance-taking
  matrix APIs.
- Add regression coverage for invalid tolerance construction and symmetry
  tolerance overflow.
- Update exact examples to propagate typed crate errors instead of unwrapping.

Closes #94
@acgetchell acgetchell enabled auto-merge June 3, 2026 08:09
@acgetchell acgetchell self-assigned this Jun 3, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR enforces a unified tolerance validation contract across the crate. Tolerance parameters must be finite and non-negative; negative, NaN, and infinite values are explicitly rejected. Core library validation and tests establish the contract; matrix symmetry, decomposition, and vector operation methods document the requirement; examples convert to Result-based error handling.

Changes

Tolerance validation contract enforcement

Layer / File(s) Summary
Tolerance validation contract and core library tests
src/lib.rs
Documentation updates clarify the crate-wide tolerance contract (finite, non-negative). New unit tests cover Tolerance::new acceptance for valid inputs (0.0, 1e-12, f64::MAX), rejection for negative/NaN/infinity with LaError::InvalidTolerance, and parity with LaError::validate_tolerance for both valid and invalid values.
Matrix symmetry methods: tolerance semantics and overflow detection
src/matrix.rs
Refines is_symmetric, first_asymmetry, and symmetry_epsilon documentation to explicitly state finite, non-negative tolerance requirements. Implements runtime overflow detection in symmetry_epsilon: when scaled epsilon accumulation becomes non-finite, the method returns LaError::NonFinite with column coordinates. Adds regression test confirming scaled-epsilon overflow handling for both symmetry methods.
Matrix decomposition methods: tolerance parameter documentation
src/matrix.rs
Expands lu, ldlt, and det documentation to explicitly describe tolerance input validation behavior and the LaError::InvalidTolerance error case.
Vector operations: tolerance semantics and overflow detection
src/vector.rs
Updates dot and norm2_sq documentation to clarify f64::mul_add-based accumulation and non-finite overflow checks. Adds test cases rejecting non-finite RHS values and accumulation-time overflow-to-non-finite transitions.
Example programs: Result-based error propagation
examples/exact_det_3x3.rs, examples/exact_solve_3x3.rs
Both examples convert main from infallible to fallible Result<(), LaError>. Tolerance construction, decomposition, and determinant/solve operations propagate errors via ? instead of unwrap(). Element access switches from unchecked get() to checked get_checked(). Functions return Ok(()) explicitly.
Changelog entry
CHANGELOG.md
Documents the feature as Feat!(api): enforce fallible numeric invariants.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • acgetchell/la-stack#127: Both PRs implement the same "fallible numeric invariants" API changes—e.g., updating Vector::dot/norm2_sq (and their non-finite/overflow error behavior) to return Result and updating call sites/tests accordingly.
  • acgetchell/la-stack#124: The main PR's updates to src/matrix.rs around symmetry validation—especially symmetry_epsilon returning LaError::NonFinite and related first_asymmetry overflow tests—directly build on the same fallible invariants/error-path work introduced in PR #124.
  • acgetchell/la-stack#93: Both PRs modify src/matrix.rs logic and tests around symmetry checking—especially first_asymmetry/symmetry_epsilon and Tolerance-scaled epsilon overflow/NonFinite handling—so the changes are directly related.

Suggested labels

documentation, enhancement, rust, testing

Poem

🐰 With checks so tight and tolerances true,
Our numbers now fail fast, not sneak through,
From NaN to infinity, all caught with care,
The API's contract, now spelled out with flair!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Updates to the exact examples (examples/exact_det_3x3.rs and examples/exact_solve_3x3.rs) extend beyond the scope of issue #94, which focuses on tolerance validation across matrix APIs rather than example propagation. Either move example updates to a separate PR aligned with an issue about error propagation, or clarify how they directly support the tolerance-validation contract audit in #94.
Linked Issues check ❓ Inconclusive The PR partially addresses issue #94: it documents tolerance contracts and adds regression tests, but implements debug_assert assertions rather than the proposed fallible error handling for invalid tolerance construction. Clarify whether the PR intends to defer the typed LaError::InvalidTolerance enforcement (mentioned as stretch goal) to a future PR, or if this is the intended final implementation for #94.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(matrix): reject overflowed symmetry tolerance scaling' directly summarizes a key change in the PR—surfacing tolerance overflow errors from symmetry checks—which is evident in the matrix.rs changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/94-tolerance-invariants

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.54%. Comparing base (1dac6cc) to head (a7b052a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   99.45%   99.54%   +0.08%     
==========================================
  Files           5        5              
  Lines        2383     2412      +29     
==========================================
+ Hits         2370     2401      +31     
+ Misses         13       11       -2     
Flag Coverage Δ
unittests 99.54% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@CHANGELOG.md`:
- Line 13: The CHANGELOG.md contains a manual entry ("Feat!(api): enforce
fallible numeric invariants [`adfc33b`]") that must not be edited by hand;
revert the manual edit to CHANGELOG.md and regenerate the file using the
changelog tooling (run either `just changelog` or `just changelog-unreleased
<version>` to prepend unreleased changes), then commit the regenerated
CHANGELOG.md instead of the manual change so the entry for commit adfc33b is
produced by the tool.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97b22cbb-1621-4720-b8b2-0db5f0ee250e

📥 Commits

Reviewing files that changed from the base of the PR and between 1dac6cc and a7b052a.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • examples/exact_det_3x3.rs
  • examples/exact_solve_3x3.rs
  • src/lib.rs
  • src/matrix.rs
  • src/vector.rs

Comment thread CHANGELOG.md
### Added

- Feat!(matrix): enforce fallible matrix invariants [`e26c283`](https://github.com/acgetchell/la-stack/commit/e26c28358b2358100353b2895441b68892e92cd7)
- Feat!(api): enforce fallible numeric invariants [`adfc33b`](https://github.com/acgetchell/la-stack/commit/adfc33b945b259721bd1067e797ed2e7d4ec0e6e)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not edit CHANGELOG.md manually.

This entry should be generated via the changelog tooling, not committed as a direct file edit. Please regenerate using just changelog (or just changelog-unreleased <version> for unreleased prepend) and commit the generated result instead.

Based on learnings: "Never edit CHANGELOG.md directly - it's auto-generated from git commits. Use just changelog to regenerate or just changelog-unreleased <version> to prepend unreleased changes."

🤖 Prompt for 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.

In `@CHANGELOG.md` at line 13, The CHANGELOG.md contains a manual entry
("Feat!(api): enforce fallible numeric invariants [`adfc33b`]") that must not be
edited by hand; revert the manual edit to CHANGELOG.md and regenerate the file
using the changelog tooling (run either `just changelog` or `just
changelog-unreleased <version>` to prepend unreleased changes), then commit the
regenerated CHANGELOG.md instead of the manual change so the entry for commit
adfc33b is produced by the tool.

@acgetchell acgetchell merged commit 3ae3781 into main Jun 3, 2026
18 checks passed
@acgetchell acgetchell deleted the fix/94-tolerance-invariants branch June 3, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

audit: validate tolerance arguments consistently across the crate

1 participant