Skip to content

Conversation

@acgetchell
Copy link
Owner

@acgetchell acgetchell commented Dec 15, 2025

Modernizes tests by adding property-based tests and tests for edge cases. Replaces examples with larger sizes to demonstrate pivoting.

This change focuses on the tests and examples, improving coverage and demonstrating common usage scenarios.

Summary by CodeRabbit

  • New Features

    • Added 5×5 example programs for determinant calculation and solving linear systems.
  • Documentation

    • Expanded README intro and quickstart to 5×5 examples; added crate-level doctest and updated example run instructions.
    • Updated spellings/wordlist entries.
  • Tests

    • Added extensive unit and property-based tests, using macro-generated suites across multiple dimensions.
  • Chores

    • Updated development dependencies, CI/build targets, and repository ignore entries.

✏️ Tip: You can customize this high-level summary in your review settings.

Modernizes tests by adding property-based tests and tests for edge
cases. Replaces examples with larger sizes to demonstrate pivoting.

This change focuses on the tests and examples, improving coverage and
demonstrating common usage scenarios.
@coderabbitai
Copy link

coderabbitai bot commented Dec 15, 2025

Walkthrough

Adds new 5×5 examples, replaces 3×3 examples, adds dev-dependencies (proptest, pastey), expands and macro-parameterizes unit tests across src/, adds property-based tests for Matrix and Vector, updates docs/CI/metadata and .gitignore, and extends crate-level doctests and unit tests.

Changes

Cohort / File(s) Summary
Repo metadata & ignores
\.gitignore, Cargo.toml, cspell.json
.gitignore patterns adjusted (use coverage/, add build*, cobertura.xml, .DS_Store); Cargo.toml added readme field, bumped dev-deps, added pastey and proptest; cspell.json added new words.
CI / Task runner
justfile
CI/test targets updated: ci now depends on lint test-all bench-compile; commit-check and examples targets updated to run 5×5 examples instead of 3×3.
Documentation
README.md, WARP.md
README quickstart and examples switched from 3×3 to 5×5, license badge path made relative; WARP examples list updated to include det_5x5 and solve_5x5.
Examples removed
examples/det_3x3.rs, examples/solve_3x3.rs
Deleted 3×3 example programs.
Examples added
examples/det_5x5.rs, examples/solve_5x5.rs
Added new 5×5 example programs computing determinant and solving linear systems using LU with pivoting.
Crate root & doctests
src/lib.rs
Added crate-level docs by including README.md, introduced cfg(doc) doctest example for 5×5, and added several unit tests (DEFAULT_PIVOT_TOL, LaError Display/Error traits, small LU flow).
LU tests
src/lu.rs
Replaced ad-hoc assertions with many macro-generated tests covering LU pivoting, det, solve across dimensions (including edge cases: singular, non-finite, pivot sign), and larger SPD tridiagonal cases.
Matrix tests
src/matrix.rs
Replaced per-dimension tests with macro-based generation (pastey) validating from_rows/get, bounds, zero, inf_norm, identity, and LU-related behaviors across D=2..5 using approx assertions.
Vector tests
src/vector.rs
Replaced inline tests with macro-generated multi-dimension tests (D=2..5) covering new/into_array, zero/default, dot and norm2_sq; added black_box and approx usage.
Property-based tests
tests/proptest_matrix.rs, tests/proptest_vector.rs
Added proptest suites: strategies and macros generating tests for dimensions 2..5 covering roundtrip, bounds, inf-norm, determinant on diagonal matrices, solve correctness, dot properties, and norm relations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Inspect macro expansions in src/lu.rs, src/matrix.rs, and src/vector.rs to ensure generated test names and assertions are correct.
  • Review new proptest strategies and case counts in tests/proptest_*.rs for edge-case coverage and stability.
  • Verify Cargo.toml dev-dependency versions and readme entry; ensure examples referenced in justfile, README.md, and WARP.md are consistent.
  • Spot-check doctest in src/lib.rs for correctness and non-flaky assertions.

Poem

🐰 I hopped from three to five with cheer,
Tests multiplied, both far and near,
Macros stitched the cases tight,
Proptest pranced into the night,
New examples nibble carrots bright. 🥕

Pre-merge checks and finishing touches

✅ 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 clearly summarizes the main change: overhauling public API tests and examples, which aligns with the widespread changes across test files, example files, and documentation updates.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tests/public-api

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5699ac5 and 9665d94.

📒 Files selected for processing (4)
  • WARP.md (1 hunks)
  • src/lu.rs (1 hunks)
  • src/vector.rs (1 hunks)
  • tests/proptest_vector.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • WARP.md
  • src/lu.rs
🧰 Additional context used
🧬 Code graph analysis (1)
tests/proptest_vector.rs (1)
src/vector.rs (6)
  • new (22-24)
  • as_array (53-55)
  • into_array (70-72)
  • dot (87-95)
  • norm2_sq (109-111)
  • zero (37-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build (macos-latest)
  • GitHub Check: build (windows-latest)
🔇 Additional comments (4)
src/vector.rs (1)

130-232: Well-structured macro-based test generation.

The macro approach effectively generates dimension-specific tests covering the public API. The use of black_box on values and function pointers is a clever technique to ensure coverage tools attribute execution to the actual loop body rather than optimized-away constants. The expected value computation mirrors the implementation's mul_add pattern, which is appropriate for validating behavior.

tests/proptest_vector.rs (3)

9-11: Appropriate strategy for avoiding floating-point edge cases.

The small_f64() strategy constrains values to [-100.0, 100.0] with one decimal precision, avoiding denormals, infinities, and extreme magnitudes that could obscure the properties under test. This is a reasonable trade-off for these foundational invariant checks.


13-60: Solid property-based test coverage.

The generated proptests validate important mathematical invariants:

  • Roundtrip consistency for new/as_array/into_array
  • Dot product commutativity with appropriate tolerance for accumulation order differences
  • norm2_sq() identity with dot(self, self) (correctly uses epsilon = 0.0 since both call the same code path)
  • Non-negativity of squared norm
  • Zero vector identity

The proptest::array::[<uniform $d>] pattern correctly expands to uniform2, uniform3, etc., which are provided by proptest's array module.


62-66: LGTM!

The macro invocations for dimensions 2-5 align with the unit test coverage in src/vector.rs, providing consistent multi-dimension validation across both deterministic and property-based test suites.


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.

❤️ Share

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

@codecov
Copy link

codecov bot commented Dec 15, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Copy link

@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

🧹 Nitpick comments (3)
WARP.md (1)

24-24: Docs: example reference now correctly targets the 5×5 determinant demo

The updated cargo run --example det_5x5 matches the new example and the examples recipe. Optionally, you could also mention solve_5x5 here for symmetry with the just examples target, but not required.

src/lu.rs (1)

183-260: Pivoting public-API tests look solid; RHS generation could be made dimension-agnostic

These macro-generated tests nicely exercise the public paths Matrix::luLu::solve_vec/Lu::det, use black_box on both the function pointers and data, and correctly validate:

  • Row-swap pivoting for a permutation matrix that swaps the first two basis vectors.
  • That solve_vec effectively inverts this permutation on a simple RHS.

One small maintainability tweak: values = [1.0, 2.0, 3.0, 4.0, 5.0] is hard-coded to length 5. Today you only instantiate the macro for D ∈ {2,3,4,5}, so it’s fine, but if someone later extends it to larger D, the tail entries of b_arr will remain 0.0.

You could make this dimension-agnostic with something like:

let b_arr = {
    let mut arr = [0.0f64; $d];
    for (i, dst) in arr.iter_mut().enumerate() {
        *dst = (i + 1) as f64;
    }
    arr
};

so the macro remains correct for any future $d.

tests/proptest_vector.rs (1)

13-60: Consider using a small epsilon for dot product comparisons.

The property-based tests are well-designed and cover important mathematical properties. However, lines 45 and 55 use epsilon = 0.0 for dot product comparisons. While this may work with the limited input range (-100.0 to 100.0), floating-point arithmetic can introduce rounding errors.

Consider using a small epsilon for robustness:

-                    assert_abs_diff_eq!(dot_ab, dot_ba, epsilon = 0.0);
+                    assert_abs_diff_eq!(dot_ab, dot_ba, epsilon = 1e-14);
-                    assert_abs_diff_eq!(a.dot(z), 0.0, epsilon = 0.0);
+                    assert_abs_diff_eq!(a.dot(z), 0.0, epsilon = 1e-14);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff96949 and 5699ac5.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (16)
  • .gitignore (1 hunks)
  • Cargo.toml (2 hunks)
  • README.md (5 hunks)
  • WARP.md (1 hunks)
  • cspell.json (2 hunks)
  • examples/det_3x3.rs (0 hunks)
  • examples/det_5x5.rs (1 hunks)
  • examples/solve_3x3.rs (0 hunks)
  • examples/solve_5x5.rs (1 hunks)
  • justfile (3 hunks)
  • src/lib.rs (2 hunks)
  • src/lu.rs (1 hunks)
  • src/matrix.rs (1 hunks)
  • src/vector.rs (1 hunks)
  • tests/proptest_matrix.rs (1 hunks)
  • tests/proptest_vector.rs (1 hunks)
💤 Files with no reviewable changes (2)
  • examples/solve_3x3.rs
  • examples/det_3x3.rs
🧰 Additional context used
🧬 Code graph analysis (8)
examples/det_5x5.rs (2)
src/matrix.rs (3)
  • from_rows (25-27)
  • lu (165-167)
  • det (186-188)
src/lu.rs (1)
  • det (169-175)
tests/proptest_matrix.rs (2)
tests/proptest_vector.rs (1)
  • small_f64 (9-11)
src/matrix.rs (7)
  • from_rows (25-27)
  • get (84-90)
  • zero (40-44)
  • set (107-114)
  • inf_norm (128-139)
  • det (186-188)
  • lu (165-167)
src/vector.rs (1)
src/matrix.rs (1)
  • zero (40-44)
src/matrix.rs (2)
src/vector.rs (3)
  • zero (37-39)
  • new (22-24)
  • into_array (70-72)
src/lu.rs (2)
  • det (169-175)
  • solve_vec (109-149)
examples/solve_5x5.rs (3)
examples/det_5x5.rs (1)
  • main (5-22)
src/matrix.rs (2)
  • from_rows (25-27)
  • lu (165-167)
src/vector.rs (1)
  • new (22-24)
src/lu.rs (2)
src/matrix.rs (3)
  • from_rows (25-27)
  • lu (165-167)
  • det (186-188)
src/vector.rs (2)
  • new (22-24)
  • into_array (70-72)
tests/proptest_vector.rs (1)
src/vector.rs (6)
  • new (22-24)
  • as_array (53-55)
  • into_array (70-72)
  • dot (87-95)
  • norm2_sq (109-111)
  • zero (37-39)
src/lib.rs (2)
src/matrix.rs (1)
  • identity (59-69)
src/vector.rs (1)
  • new (22-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: audit
  • GitHub Check: build (windows-latest)
  • GitHub Check: build (macos-latest)
🔇 Additional comments (29)
.gitignore (1)

3-6: Ignore patterns align with new coverage and build artifacts

/coverage matches the tarpaulin XML output, build* and cobertura.xml cover common local artifacts, and .DS_Store is a harmless quality-of-life ignore. No issues.

cspell.json (1)

9-9: Spell-check dictionary correctly extended for new tests and tooling

The added terms (doctests, laerror, nonfinite, pastey, proptest, proptests, tridiagonal) match the new test/tooling vocabulary and keep the list ordered. Looks good.

Also applies to: 18-18, 25-26, 30-31, 35-35

examples/det_5x5.rs (1)

1-22: 5×5 determinant example is mathematically and API-wise consistent

The matrix really is J - I and det(J - I) = (D - 1) * (-1)^(D-1) = 4 for D = 5, and the example exercises Matrix::<5>::from_rows, lu(DEFAULT_PIVOT_TOL), and Lu::det through the public API. No changes needed.

Cargo.toml (1)

8-8: Manifest updates are consistent with the new test/bench setup

Adding readme = "README.md" is standard, and the dev-dependency versions line up with the new macro/property-based tests and benches. Please just confirm these crate versions are compatible with rust-version = "1.92" and edition 2024 in your CI matrix.

Also applies to: 17-21

examples/solve_5x5.rs (1)

1-24: 5×5 solve example correctly demonstrates pivoting and solve_vec

A and b are consistent with the comment (x = [1,2,3,4,5]b = [14,13,12,11,10]), and a[0][0] = 0 indeed forces a pivot in column 0. The example cleanly exercises Matrix::lu, Lu::solve_vec, and Vector::<5>::new from the prelude.

justfile (1)

45-45: CI and workflow recipes now cover benches and 5×5 examples

ci and commit-check now include bench-compile, which is a nice safety net without running benches, and the examples recipe correctly runs det_5x5 and solve_5x5. Dependency ordering looks sane and non-cyclic.

Also applies to: 57-57, 103-105

src/lu.rs (2)

261-335: Tridiagonal smoke tests correctly encode the known det = D + 1 identity

The tridiagonal matrix construction (2 on the diagonal, −1 on sub/super-diagonals) is correct, and:

  • b built as [1, 0, ..., 0, 1] really corresponds to x = [1, ..., 1] for this SPD system, so asserting every x_i ≈ 1.0 is mathematically sound.
  • The determinant check det ≈ f64::from($d) + 1.0 matches the closed-form formula for this Toeplitz tridiagonal, with reasonable epsilons for D = 16, 32, 64.

These tests give good large-D coverage of both solve_vec and det without depending on nalgebra at runtime.


337-448: 1×1/2×2 basics plus singular/non-finite tests give strong edge-case coverage

The additional tests in this block look well thought out:

  • solve_1x1, solve_2x2_basic, and det_2x2_basic use black_box and function pointers to exercise both Lu::factor and the public Matrix::lu, Lu::solve_vec, and Lu::det paths.
  • det_requires_pivot_sign and solve_requires_pivoting correctly use a 2×2 permutation matrix to prove that pivoting and piv_sign are wired into the determinant and solve logic.
  • singular_detected and singular_due_to_tolerance_at_first_pivot validate both exact singularity and tolerance-based singular detection with precise pivot_col expectations.
  • The non-finite tests (NaN/∞ in pivot entry, pivot column scan, and the overflow scenarios in forward and back substitution) line up with the control-flow in factor and solve_vec, and the asserted pivot_col indices match where LaError::NonFinite is actually raised.

Overall, this is a very thorough set of behavioral tests around the LU error paths; no changes needed.

src/vector.rs (2)

125-128: LGTM!

The imports are well-chosen: black_box prevents constant-folding for better coverage metrics, approx provides robust floating-point assertions, and pastey enables macro-based test name generation.


229-232: LGTM!

The macro invocations for dimensions 2-5 provide comprehensive coverage and align with the property-based tests in tests/proptest_vector.rs.

src/lib.rs (3)

3-3: LGTM!

Including the README in crate documentation is a best practice that ensures docs.rs displays comprehensive usage information.


5-32: LGTM!

The doctest provides an excellent demonstration of LU decomposition with pivoting. The 5×5 system (J - I matrix) is mathematically correct and showcases the library's ability to handle cases where the initial pivot is zero.


90-132: LGTM!

The unit tests provide solid coverage of constants, error formatting, trait implementations, and prelude exports. The integration test at lines 123-131 ensures all prelude items work together correctly.

tests/proptest_matrix.rs (6)

9-15: LGTM!

The strategy functions provide well-bounded test inputs (-100.0 to 100.0) that avoid numerical overflow while ensuring diverse test coverage. The small_nonzero_f64 strategy correctly excludes zero for diagonal matrices.


17-40: LGTM!

The roundtrip test thoroughly validates from_rows and get, including proper boundary checking. The out-of-bounds validation at lines 38-39 ensures the API contract is enforced.


42-51: LGTM!

The set/get test correctly validates in-bounds mutation behavior. Using epsilon = 0.0 is appropriate here since the value is stored and retrieved without arithmetic operations.


53-68: LGTM!

The infinity norm test correctly validates that the implementation matches the mathematical definition (maximum absolute row sum) and enforces non-negativity.


70-100: LGTM!

The diagonal matrix test is an excellent property-based validation: the determinant is the product of diagonal elements, and the solution is element-wise division. The tolerance of 1e-12 is appropriate for LU decomposition operations.


107-110: LGTM!

The macro invocations for dimensions 2-5 provide comprehensive test coverage and align with the testing strategy across the codebase.

README.md (5)

5-5: LGTM!

The relative path ./LICENSE is more explicit and ensures proper linking across different documentation contexts.


14-15: LGTM!

The expanded introduction clearly communicates the crate's purpose and design philosophy, providing valuable context for potential users.


49-73: LGTM!

The upgraded 5×5 example effectively demonstrates LU decomposition with pivoting. The J - I matrix (zeros on diagonal) is an excellent showcase for the library's pivot handling. The mathematical correctness was verified, and the tolerance of 1e-12 is appropriate.


93-94: LGTM!

The example commands correctly reference the new 5×5 examples, maintaining consistency with the codebase updates.


111-111: LGTM!

The license link update matches the badge update at line 5, ensuring consistency throughout the document.

src/matrix.rs (3)

203-204: LGTM!

The imports are appropriate for macro-based test generation with floating-point comparisons.


206-295: LGTM!

The macro generates comprehensive tests covering construction, bounds checking, norms, and LU operations. The tolerance choices are appropriate: epsilon = 0.0 for exact values and epsilon = 1e-12 for numerical operations.


298-301: LGTM!

The macro invocations for dimensions 2-5 provide comprehensive coverage and maintain consistency with the testing strategy across the codebase.

tests/proptest_vector.rs (2)

9-11: LGTM!

The strategy function provides well-bounded test inputs that avoid numerical overflow while ensuring diverse test coverage, consistent with tests/proptest_matrix.rs.


63-66: LGTM!

The macro invocations for dimensions 2-5 provide comprehensive test coverage and maintain consistency with the testing strategy across the entire codebase.

Updates tests to use a smaller epsilon value (1e-14)
when comparing floating-point numbers, improving
numerical stability and preventing false negatives.

Also, refactors test setup in `lu.rs` for clarity.
@acgetchell acgetchell merged commit b90468e into main Dec 15, 2025
8 checks passed
@acgetchell acgetchell deleted the tests/public-api branch December 15, 2025 08:10
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.

2 participants