Faster determinant for matrices over gf2e (M4RIE)#40773
Faster determinant for matrices over gf2e (M4RIE)#40773vbraun merged 4 commits intosagemath:developfrom
Conversation
|
Documentation preview for this PR (built with commit 8ee83ca; changes) is ready! 🎉 |
|
|
||
| sig_on() | ||
| cdef int r = mzed_ple(A, P, Q) | ||
| sig_off() |
There was a problem hiding this comment.
My impression is it would make the code much easier to maintain if you just make a single method that compute the PLE decomposition, then in determinant you return prod(self.ple_decomposition()[0].diagonal()) or something. If one can accept the overhead (which is likely negligible compared to the likely O(n^3) PLE decomposition).
There was a problem hiding this comment.
Yes, I agree (including about the negligible overhead), however this requires a bit more work: already for doing it for this specific class, but also and mostly because I think there is some rewriting to do so that the general LU method calls this kind of fast PLE/PLUQ/... when available. For example matrices in modn_dense or mod2 or gf2e should be able to do LU by a direct call to FFLAS-FFPACK/M4RI/M4RIE functions for PLE/PLUQ. For the moment, Sage's LU is a naive algorithm for these classes. So I preferred to simply accelerate the determinant for now, and keep the LU improvements for a moment when I have time to do them properly.
There was a problem hiding this comment.
not necessarily so, you can just expose an additional method .ple_decomposition() and explain what it does. Making LU call it is an optional extra improvement.
But either way, this is optional.
There was a problem hiding this comment.
To remember about this comment and handle it when time permits (if no one does it before then), I have created #40791
|
|
||
| cdef Cache_base cache = <Cache_base> self._base_ring._cache | ||
|
|
||
| # characteristic 2, so det(P) == det(Q) == 1 |
There was a problem hiding this comment.
PLE decomposition has P and Q being a permutation matrix in whatever characteristic right? (plus, here P and Q aren't really matrix either, they're represented by permutations which can be converted to permutation matrix if necessary)
There was a problem hiding this comment.
Yes, well in case of PLUQ yes, for PLE (which has no Q in its definition) it depends what we mean by Q and how pivots are chosen, but here for M4RIE both are indeed permutation matrices. The comment is here because if the characteristic was not 2, the determinant of P and Q would need to be computed.
This computes the determinant for matrices over
GF(2**e)by directly relying on the PLE decomposition of M4RIE. This provides a significant speed-up over the generic implementation used until now for this class.Before:
After: