Fixed determinant_fast()
#3
Open
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.
The Algorithm's Issue
When trying to implement the matrix multiplication algorithm in this library (getting the REF (Row Echelon Form) with Gaussian Elimination and then multiplying the main diagonal for the determinant) I found the issue with
determinant_fast()that was also mentioned by issue #2.The pivot being 0 would cause
determinant_fast()to return the wrong determinant due to the lack of row swapping.The Fix
This pull request fixes that by adding row swapping like this on line 331:
alongside the
odd_swapsvariable for check whether there's been an odd number of swaps and the negator at the end for if there has been an odd number of swaps.and removing the "cheat":
from here
The Mistake
Contrary to what's said in the article:
the cheat and fancy system that includes keeping track of the number of times rows were swapped to change the sign at the end isn't required, all that's required is row swapping and a boolean to tell whether or not there's been an odd number of swaps.
Please update the article
https://integratedmlai.com/find-the-determinant-of-a-matrix-with-pure-python-without-numpy-or-scipy/
The Solution for those who are trying to use the algorithm
The author doesn't appear to be active on his github, so hopefully this'll help anyone who's confused
just copy
determinant_fast()from my pull request here: https://github.com/RandomGamingDev/BasicLinearAlgebraToolsPurePy/blob/master/LinearAlgebraPurePython.py#L315How can I contact @ThomIves
Also, I don't know how to contact the author so it'd be appreciated if someone who did know could help contact him or send his contact details to me so that we can update the article.