Repository navigation
This closes #2413, give det the determinant of a single element - #2420
Merged
Merged
Conversation
The cofactors of a 2x2 matrix are 1x1 determinants. Without a 1x1 case they fell into the Laplace expansion, which takes the minor of a 1x1 matrix, gets an empty matrix and sums over nothing, so every cofactor was 0 and MINVERSE of a 2x2 matrix was all zeros. MDETERM was not affected because `det` has its own 2x2 branch and never descends, which is why the existing tests did not catch it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: PhilflowIO <tech@philflow.io>
5 of 10 tasks
xuri
requested changes
Oct 7, 2026
xuri
left a comment
Member
There was a problem hiding this comment.
Thanks for your pull request. I've left a comment.
xuri
approved these changes
Oct 7, 2026
xuri
left a comment
Member
There was a problem hiding this comment.
Good catch. Thanks for your contribution. I've made some changes based on your code branch.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2420 +/- ##
=======================================
Coverage 99.72% 99.72%
=======================================
Files 32 32
Lines 32513 32524 +11
=======================================
+ Hits 32423 32434 +11
Misses 88 88
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Details
Description
Adds the 1x1 case to
det: the determinant of a single element is that element. Replaces the expectation"MINVERSE(A1:B2)": "-0", which pinned the wrong result, and addsTestCalcMINVERSEplus two directdetassertions.Related Issue
Closes #2413
Motivation and Context
The cofactors of a 2x2 matrix are 1x1 determinants. Without a 1x1 case they fell into the Laplace expansion, which takes the minor of a 1x1 matrix, gets an empty matrix and sums over nothing, so every cofactor was 0 and MINVERSE of a 2x2 matrix was all zeros. MDETERM was not affected because
dethas its own 2x2 branch and never descends, which is why the existing tests did not catch it.How Has This Been Tested
TestCalcMINVERSEchecks every element of the inverse of a 2x2 and a 3x3 matrix, not only the top left one, and the determinant of both.TestCalcDetcallsdetdirectly for a 1x1 and a 2x2 matrix.go test ./...passes on go1.26.8 linux/amd64.gofmt -s -l .is empty.Types of changes
Checklist