Skip to content

Support floating-point determinants in matrix inversion - #2781

Open
pralav-25 wants to merge 1 commit into
keon:mainfrom
pralav-25:fix_float_matrix_inverse
Open

pralav-25 wants to merge 1 commit into
keon:mainfrom
pralav-25:fix_float_matrix_inverse

Conversation

@pralav-25

@pralav-25 pralav-25 commented Sep 27, 2026 •

Copy link
Copy Markdown

Inverting an otherwise valid 3×3 floating-point matrix raises TypeError because Fraction(1, determinant) requires rational arguments. Dividing Fraction(1) by the determinant supports floats while retaining exact rational results for integer determinants.

Adds two-sided identity checks for 2×2, 3×3 and 4×4 matrices, verifies input preservation, and covers exact Fraction results and the existing singular-matrix sentinel.

Validation on Python 3.12.14:

  • The new test file reproduced 4 failing cases on unmodified main.
  • python -m pytest -q: 607 passed.
  • python -m pytest --doctest-modules algorithms/ -q: 480 passed, 1 skipped.
  • ruff check algorithms/ tests/ and git diff --check: passed.

Prepared with AI assistance; the failures and fixes above were reproduced locally.

Summary by CodeRabbit

  • Bug Fixes
    • Improved inversion of matrices larger than 2×2 when working with numeric values, including floats.
  • Tests
    • Added checks for inversion accuracy, exact results with integer matrices, input preservation, and singular-matrix handling.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 609ba847-701b-4baf-bc33-a7d5af5e7e99

📥 Commits

Reviewing files that changed from the base of the PR and between bd1d152 and 98b4eab.

📒 Files selected for processing (2)
  • algorithms/matrix/matrix_inversion.py
  • tests/test_matrix_inverse_numeric_types.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

For matrices with more than two rows, invert_matrix now calculates the reciprocal determinant using division. New tests cover float matrix inverses, exact Fraction results for integer matrices, input preservation, and the singular-matrix sentinel.

Changes

Matrix inversion numeric types

Layer / File(s) Summary
Determinant multiplier and numeric-type coverage
algorithms/matrix/matrix_inversion.py, tests/test_matrix_inverse_numeric_types.py
The larger-matrix path divides Fraction(1) by the determinant. Tests check float inverse identity in both multiplication orders, input preservation, exact Fraction results for integer matrices, and the singular float-matrix sentinel.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: keon

Merge Risk: ⚪ Minimal · up to 98b4e

The update adds floating-point inversion coverage while retaining exact integer results. No concrete merge-blocking regression is indicated.

🚥 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 and concisely describes the main change: supporting floating-point determinants in matrix inversion.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

This branch has not been deployed

No deployments
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.

1 participant