Skip to content

refactor(exact): encode nonzero mantissas in exact decomposition#130

Merged
acgetchell merged 1 commit into
mainfrom
refactor/120-nonzero-exact-mantissas
Jun 3, 2026
Merged

refactor(exact): encode nonzero mantissas in exact decomposition#130
acgetchell merged 1 commit into
mainfrom
refactor/120-nonzero-exact-mantissas

Conversation

@acgetchell
Copy link
Copy Markdown
Owner

@acgetchell acgetchell commented Jun 3, 2026

  • Replace the exact-arithmetic zero mantissa sentinel with Option<NonZeroU64>.
  • Carry nonzero mantissa proof through matrix/vector decomposition and BigInt scaling.
  • Clarify determinant documentation around uncertified det() bounds.
  • Keep SPD determinant proptests on the tolerance-aware LU path.

Closes #120

Summary by CodeRabbit

  • Documentation

    • Updated README image URL to a GitHub-hosted source
    • Clarified Matrix::det() behavior documentation regarding singularity detection and fallback logic
  • Refactor

    • Updated internal mantissa representation in exact arithmetic component handling
  • Tests

    • Updated determinant validation tests to improve accuracy

- Replace the exact-arithmetic zero mantissa sentinel with `Option<NonZeroU64>`.
- Carry nonzero mantissa proof through matrix/vector decomposition and BigInt scaling.
- Clarify determinant documentation around uncertified `det()` bounds.
- Keep SPD determinant proptests on the tolerance-aware LU path.

Closes #120
@acgetchell acgetchell self-assigned this Jun 3, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5cf189e-3b5f-4a8a-8c76-35d965c60427

📥 Commits

Reviewing files that changed from the base of the PR and between 18a4e44 and 7a664ed.

📒 Files selected for processing (4)
  • README.md
  • src/exact.rs
  • src/matrix.rs
  • tests/proptest_matrix.rs

📝 Walkthrough

Walkthrough

This PR refactors exact arithmetic mantissa representation from raw u64 to Option<NonZeroU64>, making zero invariants unrepresentable and eliminating sentinel checks. It updates f64_decompose, Component, and conversion logic throughout, adds test coverage for the refined type, and clarifies Matrix::det() singularity semantics with an updated proptest reference.

Changes

Exact Arithmetic NonZero Mantissa Refactoring and Determinant Behavior Clarification

Layer / File(s) Summary
f64_decompose signature and NonZeroU64 type contract
src/exact.rs
NonZeroU64 import added; f64_decompose function signature updated to return Option<(NonZeroU64, i32, bool)>, guaranteeing mantissa non-zero when present.
Component struct internal representation and decomposition helpers
src/exact.rs
Internal Component struct refactored from mantissa: u64 zero-sentinel to mantissa: Option<NonZeroU64>; mantissa values wrapped as Some(NonZeroU64) in decompose_matrix and decompose_vec; zero entries produce Component::default().
component_to_bigint conversion refactoring
src/exact.rs
component_to_bigint reworked to use Option-based branching: NoneBigInt::from(0), Some(mantissa) → unwrap via mantissa.get() and apply existing sign/exponent scaling; test helper f64_to_bigrational updated for NonZeroU64 conversion.
Exact arithmetic decomposition and conversion test updates
src/exact.rs
All f64_decompose unit tests (f64_decompose_one, f64_decompose_negative, f64_decompose_subnormal, f64_decompose_power_of_two) updated to assert mantissa via mant.get(); new test component_to_bigint_distinguishes_zero_from_nonzero_mantissa validates zero vs. non-zero mantissa branching for both positive and negative signs.
Determinant behavior documentation and proptest reference update
src/matrix.rs, tests/proptest_matrix.rs
Matrix::det() Rustdoc clarified regarding singularity handling, det_direct() fast paths, and LU fallback interaction; SPD ldlt property test switched from a.det().unwrap() to a.lu(DEFAULT_PIVOT_TOL).unwrap().det().unwrap() for determinant reference computation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • acgetchell/la-stack#120: PR directly implements the parse-dont-validate NonZero audit by replacing raw u64 mantissa sentinel with Option, moving zero validation to decomposition construction.

Possibly related PRs

  • acgetchell/la-stack#96: Both PRs modify src/exact.rs f64_decompose and component mantissa representation; Bareiss-based integer elimination and determinant logic would need to adapt to the new NonZeroU64-wrapped Option behavior.
  • acgetchell/la-stack#73: Both PRs change f64_decompose floating-point decomposition path and downstream component/matrix conversion logic for mantissa handling.
  • acgetchell/la-stack#129: Main PR's Matrix::det() semantics/documentation refinement and proptest determinant reference adjustment directly align with det behavior clarifications introduced in PR #129.

Suggested labels

rust, testing, documentation, enhancement

Poem

🐰 A mantissa now wears NonZero's coat,
No sentinel tricks—just truth by rote,
Components sing with Option's grace,
Determinants dance in their proper place,
Parse-don't-validate wins the race! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: refactoring the exact decomposition to use NonZeroU64 for mantissas, matching the main objective of the PR.
Linked Issues check ✅ Passed The PR fulfills issue #120 by replacing zero-mantissa sentinels with Option, propagating the nonzero proof through decomposition logic, and updating tests accordingly.
Out of Scope Changes check ✅ Passed All changes directly support the PR objectives: NonZeroU64 mantissa encoding, documentation clarification, and test updates for determinant paths are all within scope.
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 refactor/120-nonzero-exact-mantissas

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

@acgetchell acgetchell enabled auto-merge June 3, 2026 16:18
@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.56%. Comparing base (18a4e44) to head (7a664ed).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #130   +/-   ##
=======================================
  Coverage   99.55%   99.56%           
=======================================
  Files           5        5           
  Lines        2497     2517   +20     
=======================================
+ Hits         2486     2506   +20     
  Misses         11       11           
Flag Coverage Δ
unittests 99.56% <100.00%> (+<0.01%) ⬆️

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.

@acgetchell acgetchell merged commit d66a43a into main Jun 3, 2026
18 checks passed
@acgetchell acgetchell deleted the refactor/120-nonzero-exact-mantissas branch June 3, 2026 16:24
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.

Run parse-dont-validate NonZero audit

1 participant