Open Makogan opened 3 months ago
So this is basically the same as the method taught in school for manual solving of matrices. Should we be using something like QR decomposition with Givens rotations instead which is much more stable under numerical issues that show up from poorly conditioned matrices? Such as Matlab does when you use the solve command? https://en.wikipedia.org/wiki/QR_decomposition
https://en.wikipedia.org/wiki/Gaussian_elimination#Gauss%E2%80%93Jordan_elimination I think is close to what is currently being used. Another benefit would be that Givens rotations are row-independent and therefor can be parallelized easily.
although LU factorization is faster, so maybe should be used instead of QR. https://search.brave.com/search?q=how+does+lapack+invert+matrix&source=desktop
or maybe it is up to the user to use a numerically strong method if they need one.
Specifically here.
A determinant close to 0 would still cause numerical issues that can result in NAN's.
I found out the hard way with his snippet:
And this input matrix:
Using a heuristic, the determinant of this matrix is about
8.26×10 −38
Which is essentially 0, however since it is not identically 0, the linked code proceeds to compute multiple divisions which yield NAN's and infs, so the resulting matrix is useless.On it's current form the method is problematic, as a user would expect that if
try_inverse
succeeds, then the result is sane.Possible fixes:
1) Add an epsilon parameter to the signature to define the failure case for the determinant, rather than test against 0. 2) Add a post computation check that returns None if any of the entries are NaNs/infs 3) Add a
debug_assert
that crashes the application if any entires are Nans but does not add overhead for release builds.