Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/sage/libs/m4rie.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# http://www.gnu.org/licenses/
##############################################################################

from sage.libs.m4ri cimport mzd_t, m4ri_word
from sage.libs.m4ri cimport mzd_t, m4ri_word, mzp_t


cdef extern from "m4rie/m4rie.h":
Expand Down Expand Up @@ -109,6 +109,8 @@ cdef extern from "m4rie/m4rie.h":

size_t mzed_echelonize_ple(mzed_t *, size_t)

int mzed_ple(mzed_t *A, mzp_t *P, mzp_t *Q)

#cdef extern from "m4rie/strassen.h":
mzed_t *mzed_mul_strassen(mzed_t *, mzed_t *, mzed_t *, size_t cutoff)

Expand Down
84 changes: 83 additions & 1 deletion src/sage/matrix/matrix_gf2e_dense.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
from sage.matrix.matrix_mod2_dense cimport Matrix_mod2_dense
from sage.matrix.args cimport SparseEntry, MatrixArgs_init

from sage.libs.m4ri cimport m4ri_word, mzd_copy
from sage.libs.m4ri cimport m4ri_word, mzd_copy, mzp_t, mzp_init, mzp_free
from sage.libs.m4rie cimport *
from sage.libs.m4rie cimport mzed_t

Expand Down Expand Up @@ -1010,7 +1010,7 @@

sage: K.<a> = GF(2^3)
sage: A = random_matrix(K, 3, 3)
sage: while A.rank() != 3:

Check warning on line 1013 in src/sage/matrix/matrix_gf2e_dense.pyx

View workflow job for this annotation

GitHub Actions / Conda (ubuntu, Python 3.11, all)

Warning: slow doctest:

slow doctest:: Test ran for 8.95s cpu, 11.18s wall Check ran for 0.00s cpu, 0.00s wall

Check warning on line 1013 in src/sage/matrix/matrix_gf2e_dense.pyx

View workflow job for this annotation

GitHub Actions / Conda (ubuntu, Python 3.12, all)

Warning: slow doctest:

slow doctest:: Test ran for 9.34s cpu, 11.42s wall Check ran for 0.00s cpu, 0.00s wall

Check warning on line 1013 in src/sage/matrix/matrix_gf2e_dense.pyx

View workflow job for this annotation

GitHub Actions / Conda (ubuntu, Python 3.12, all, editable)

Warning: slow doctest:

slow doctest:: Test ran for 8.99s cpu, 10.48s wall Check ran for 0.00s cpu, 0.00s wall

Check warning on line 1013 in src/sage/matrix/matrix_gf2e_dense.pyx

View workflow job for this annotation

GitHub Actions / Conda (macos, Python 3.12, all)

Warning: slow doctest:

slow doctest:: Test ran for 5.61s cpu, 9.91s wall Check ran for 0.00s cpu, 0.00s wall
....: A = random_matrix(K, 3, 3)
sage: B = ~A
sage: A*B
Expand Down Expand Up @@ -1586,6 +1586,88 @@
mzed_cling(self._entries, v)
mzd_slice_free(v)

def determinant(self):
"""
Return the determinant of this matrix.

Relies directly on M4RIE's PLE decomposition, and incidentally caches
the rank of ``self``.

EXAMPLES::

sage: gf4.<z> = GF(4)
sage: mat = matrix(gf4, 2, 2, [[z + 1, z + 1], [z, 1]])
sage: mat
[z + 1 z + 1]
[ z 1]
sage: mat.determinant()
z
sage: gf256.<t> = GF(2**8)
sage: mat = matrix(gf256, 3, 3, [[1, t, t**2],
....: [t**2, 1, t],
....: [t, t**2, 1]])
sage: mat.determinant()
t^6 + 1
sage: mat = matrix(gf256, 3, 3, [[1, t, t**2],
....: [t**2, 1, t],
....: [t**2 + 1, t + 1, t**2 + t]])
sage: mat.determinant()
0

Non-square matrices and the `0 \times 0` matrix are taken care of::

sage: matrix(gf4, 0, 0).determinant()
1
sage: matrix(gf4, 3, 2).determinant()
Traceback (most recent call last):
...
ValueError: self must be a square matrix
"""
cdef size_t m = self._nrows

if m != self._ncols:
raise ValueError("self must be a square matrix")
if m == 0:
return self._one

x = self.fetch('det')
if x is not None:
return x

cdef mzed_t * A = mzed_copy(NULL, self._entries)
cdef mzp_t * P = mzp_init(m)
cdef mzp_t * Q = mzp_init(m)

sig_on()
cdef int r = mzed_ple(A, P, Q)
sig_off()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@user202729 user202729 Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To remember about this comment and handle it when time permits (if no one does it before then), I have created #40791


self.cache('rank', r)

if r < m:
mzp_free(P)
mzp_free(Q)
mzed_free(A)
self.cache('det', self._zero)
return self._zero

cdef Cache_base cache = <Cache_base> self._base_ring._cache

# characteristic 2, so det(P) == det(Q) == 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

cdef Py_ssize_t i
cdef int elt
cdef det = self._one
for i from 0 <= i < m:
elt = mzed_read_elem(A, i, i)
det = det * cache.fetch_int(elt)

mzp_free(P)
mzp_free(Q)
mzed_free(A)

self.cache('det', det)
return det


def unpickle_matrix_gf2e_dense_v0(Matrix_mod2_dense a, base_ring, nrows, ncols):
r"""
Expand Down
Loading