Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Matrix_mod2_dense solve_right performance #38843

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 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
3 changes: 3 additions & 0 deletions src/sage/libs/m4ri.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ cdef extern from "m4ri/m4ri.h":
# reduced row echelon form using PLUQ factorization
cdef mzd_t *mzd_kernel_left_pluq(mzd_t *A, int cutoff)

# system solving
cdef int mzd_solve_left(mzd_t *A, mzd_t *B, int cutoff, int inconsistency_check)

########################
# Bit operations
########################
Expand Down
75 changes: 75 additions & 0 deletions src/sage/matrix/matrix_mod2_dense.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,81 @@ cdef class Matrix_mod2_dense(matrix_dense.Matrix_dense): # dense or sparse
self.cache('rank', r)
return r

def _solve_right_general(self, B, check=True):
"""
Solve the matrix equation AX = B for X using the M4RI library.

INPUT:

- ``B`` -- a matrix
- ``check`` -- boolean (default: ``True``); whether to check if the
matrix equation has a solution

EXAMPLES::

sage: A = matrix(GF(2), [[1, 0], [0, 1], [1, 1]])
sage: A.solve_right(vector([1, 1, 0]))
(1, 1)
sage: A.solve_right(vector([1, 1, 1]))
Traceback (most recent call last):
...
ValueError: matrix equation has no solutions

TESTS::

sage: n = 128
sage: m = 128
sage: A = random_matrix(GF(2), n, m)
sage: B = A * random_vector(GF(2), m)
sage: A * A.solve_right(B) == B
True
sage: m = 64
sage: A = random_matrix(GF(2), n, m)
sage: B = A * random_vector(GF(2), m)
sage: A * A.solve_right(B) == B
True
sage: m = 256
sage: A = random_matrix(GF(2), n, m)
sage: B = A * random_vector(GF(2), m)
sage: A * A.solve_right(B) == B
True
"""
cdef mzd_t *B_entries = (<Matrix_mod2_dense>B)._entries

cdef Matrix_mod2_dense X # the solution
X = self.new_matrix(nrows=self._entries.ncols, ncols=B_entries.ncols)
if self._entries.ncols == 0 or B_entries.ncols == 0:
# special case: empty matrix
if B != 0:
raise ValueError("matrix equation has no solutions")
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely this is redundant (?) if B has no entries then every entries are 0?

(also even if it can happen you forget to free X here)

Copy link
Author

@maple3142 maple3142 Oct 24, 2024

Choose a reason for hiding this comment

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

There are cases where A have zero cols but B have non zero cols, so it would have a different result depends on whether B is zero. And without this, some sage tests will fail.

As for X, I think it is a Python object so it would be handled by Python's refcount?

return X
cdef rci_t rows = self._entries.nrows
if self._entries.nrows < self._entries.ncols:
rows = self._entries.ncols # mzd_solve_left requires ncols <= nrows

cdef mzd_t *lhs = mzd_init(rows, self._entries.ncols)
mzd_copy(lhs, self._entries)
cdef mzd_t *rhs = mzd_init(rows, B_entries.ncols)
mzd_copy(rhs, B_entries)

cdef int ret
try:
sig_on()
# although it is called mzd_solve_left, it does the same thing as solve_right
ret = mzd_solve_left(lhs, rhs, 0, check)
sig_off()

if ret == 0:
# solution is placed in rhs
rhs.nrows = self._entries.ncols
mzd_copy(X._entries, rhs)
return X
else:
raise ValueError("matrix equation has no solutions")
finally:
mzd_free(lhs)
mzd_free(rhs)

def _right_kernel_matrix(self, **kwds):
r"""
Return a pair that includes a matrix of basis vectors
Expand Down
Loading