From a872aacc3f879472a98de7796b0c48c0e6d39899 Mon Sep 17 00:00:00 2001 From: Giacomo Pope Date: Tue, 4 Jun 2024 17:42:54 +0100 Subject: [PATCH 1/3] fix slow columns --- src/sage/matrix/matrix_mod2_dense.pyx | 41 +++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/sage/matrix/matrix_mod2_dense.pyx b/src/sage/matrix/matrix_mod2_dense.pyx index 0d3c15be395..1bbb41fe205 100644 --- a/src/sage/matrix/matrix_mod2_dense.pyx +++ b/src/sage/matrix/matrix_mod2_dense.pyx @@ -526,6 +526,47 @@ cdef class Matrix_mod2_dense(matrix_dense.Matrix_dense): # dense or sparse mzd_submatrix(z._entries, self._entries, i, 0, i+1, self._ncols) return z + def columns(self, copy=True): + """ + Return list of the columns of self. + + INPUT: + + - ``copy`` - (default: True) if True, return a copy so you can + modify it safely + + EXAMPLES: + + An example with a small 3x3 matrix:: + + sage: M2 = Matrix(GF(2), [[1, 0, 0], [0, 1, 0], [0, 1, 1]]) + sage: M2.columns() + [(1, 0, 0), (0, 1, 1), (0, 0, 1)] + + """ + x = self.fetch('columns') + if x is not None: + if copy: return list(x) + return x + cdef Py_ssize_t i + + # Note: due to the way M4ri represents values, extracting rows + # if fast, but columns are slow. Therefore we transpose + # then take rows. For more information, see the issue + # https://github.com/sagemath/sage/issues/38150 + C = self.transpose().rows() + + # Make the vectors immutable since we are caching them + for x in C: + x.set_immutable() + + # cache result + self.cache('columns', C) + if copy: + return list(C) + else: + return C + ######################################################################## # LEVEL 2 functionality # * def _pickle From 74b12f423b71a7dd9b34b0bce7d134a0bb0d5ad5 Mon Sep 17 00:00:00 2001 From: Giacomo Pope <44242839+GiacomoPope@users.noreply.github.com> Date: Tue, 4 Jun 2024 17:55:40 +0100 Subject: [PATCH 2/3] fix a typo in comment --- src/sage/matrix/matrix_mod2_dense.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/matrix/matrix_mod2_dense.pyx b/src/sage/matrix/matrix_mod2_dense.pyx index 1bbb41fe205..14eb6f02539 100644 --- a/src/sage/matrix/matrix_mod2_dense.pyx +++ b/src/sage/matrix/matrix_mod2_dense.pyx @@ -551,7 +551,7 @@ cdef class Matrix_mod2_dense(matrix_dense.Matrix_dense): # dense or sparse cdef Py_ssize_t i # Note: due to the way M4ri represents values, extracting rows - # if fast, but columns are slow. Therefore we transpose + # is fast, but columns are slow. Therefore we transpose # then take rows. For more information, see the issue # https://github.com/sagemath/sage/issues/38150 C = self.transpose().rows() From 6d3a2336c39f088c43da6718f342f8fd66436c48 Mon Sep 17 00:00:00 2001 From: Giacomo Pope <44242839+GiacomoPope@users.noreply.github.com> Date: Wed, 5 Jun 2024 14:58:43 +0100 Subject: [PATCH 3/3] Apply suggestions from code review Add reviewer suggestions Co-authored-by: grhkm21 <83517584+grhkm21@users.noreply.github.com> --- src/sage/matrix/matrix_mod2_dense.pyx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/sage/matrix/matrix_mod2_dense.pyx b/src/sage/matrix/matrix_mod2_dense.pyx index 14eb6f02539..5eb71dc252a 100644 --- a/src/sage/matrix/matrix_mod2_dense.pyx +++ b/src/sage/matrix/matrix_mod2_dense.pyx @@ -532,7 +532,7 @@ cdef class Matrix_mod2_dense(matrix_dense.Matrix_dense): # dense or sparse INPUT: - - ``copy`` - (default: True) if True, return a copy so you can + - ``copy`` -- (default: ``True``) if True, return a copy so you can modify it safely EXAMPLES: @@ -542,7 +542,6 @@ cdef class Matrix_mod2_dense(matrix_dense.Matrix_dense): # dense or sparse sage: M2 = Matrix(GF(2), [[1, 0, 0], [0, 1, 0], [0, 1, 1]]) sage: M2.columns() [(1, 0, 0), (0, 1, 1), (0, 0, 1)] - """ x = self.fetch('columns') if x is not None: @@ -564,8 +563,7 @@ cdef class Matrix_mod2_dense(matrix_dense.Matrix_dense): # dense or sparse self.cache('columns', C) if copy: return list(C) - else: - return C + return C ######################################################################## # LEVEL 2 functionality