Skip to content

Commit 3c7f6ba

Browse files
author
Release Manager
committed
gh-40435: Add faster implementations of matrix_from_* constructors to GF(2) and GF(2^e) Dense matrices over GF(2) and GF(2^e) currently have slow performance for the following methods: - matrix_from_rows() - matrix_from_columns() - matrix_from_rows_and_columns() They are using the default implementation in matrix1.pyx which involves conversion of each element every time it is read or written. This PR puts direct naive implementations for these methods into both classes. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [X] The title is concise and informative. - [X] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [X] I have updated the documentation and checked the documentation preview. URL: #40435 Reported by: Biffo89 Reviewer(s): Xavier Caruso
2 parents 45e6ea4 + 839f37d commit 3c7f6ba

21 files changed

+703
-7
lines changed

src/sage/matrix/matrix0.pxd

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ cdef class Matrix(sage.structure.element.Matrix):
4848
# Unsafe entry access
4949
cdef set_unsafe(self, Py_ssize_t i, Py_ssize_t j, object x)
5050
cdef get_unsafe(self, Py_ssize_t i, Py_ssize_t j)
51+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc)
5152
cdef _coerce_element(self, x)
5253
cdef bint get_is_zero_unsafe(self, Py_ssize_t i, Py_ssize_t j) except -1
5354

src/sage/matrix/matrix0.pyx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,28 @@ cdef class Matrix(sage.structure.element.Matrix):
556556
"""
557557
raise NotImplementedError("this must be defined in the derived type.")
558558

559+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc):
560+
"""
561+
Copy element (iSrc, jSrc) from ``src`` to position (iDst, jDst) in
562+
``self``. It is assumed ``src`` is the same type of matrix as``self``,
563+
with the same base ring.
564+
565+
This should generally be reimplemented in subclasses to avoid the type
566+
conversion that often is necessary in ``get_unsafe`` and
567+
``set_unsafe``.
568+
569+
INPUT:
570+
571+
- ``iDst`` - the row to be copied to in ``self``.
572+
- ``jDst`` - the column to be copied to in ``self``.
573+
- ``src`` - the matrix to copy from. Should be the same type as
574+
``self`` with the same base ring.
575+
- ``iSrc`` - the row to be copied from in ``src``.
576+
- ``jSrc`` - the column to be copied from in ``src``.
577+
"""
578+
cdef Matrix _src = <Matrix>src
579+
self.set_unsafe(iDst, jDst, _src.get_unsafe(iSrc, jSrc))
580+
559581
cdef bint get_is_zero_unsafe(self, Py_ssize_t i, Py_ssize_t j) except -1:
560582
"""
561583
Return 1 if the entry ``(i, j)`` is zero, otherwise 0.

src/sage/matrix/matrix1.pyx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,7 +2043,7 @@ cdef class Matrix(Matrix0):
20432043
if col < 0 or col >= self._ncols:
20442044
raise IndexError("column index out of range")
20452045
for i in range(self._nrows):
2046-
A.set_unsafe(i, j, self.get_unsafe(i, col))
2046+
A.copy_from_unsafe(i, j, self, i, col)
20472047
return A
20482048

20492049
def delete_columns(self, dcols, check=True):
@@ -2141,7 +2141,7 @@ cdef class Matrix(Matrix0):
21412141
if row < 0 or row >= self._nrows:
21422142
raise IndexError("row index out of range")
21432143
for j in range(self._ncols):
2144-
A.set_unsafe(i, j, self.get_unsafe(row, j))
2144+
A.copy_from_unsafe(i, j, self, row, j)
21452145
return A
21462146

21472147
def delete_rows(self, drows, check=True):
@@ -2268,7 +2268,7 @@ cdef class Matrix(Matrix0):
22682268
if row < 0 or row >= self._nrows:
22692269
raise IndexError("row index out of range")
22702270
for j, col in enumerate(columns):
2271-
A.set_unsafe(i, j, self.get_unsafe(row, col))
2271+
A.copy_from_unsafe(i, j, self, row, col)
22722272
return A
22732273

22742274
def submatrix(self, Py_ssize_t row=0, Py_ssize_t col=0,

src/sage/matrix/matrix_complex_ball_dense.pyx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,53 @@ cdef class Matrix_complex_ball_dense(Matrix_dense):
307307
acb_set(z.value, acb_mat_entry(self.value, i, j))
308308
return z
309309

310+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc):
311+
"""
312+
Copy the ``(iSrc, jSrc)`` entry of ``src`` into the ``(iDst, jDst)``
313+
entry of ``self``.
314+
315+
.. warning::
316+
317+
This is very unsafe; it assumes ``iSrc``, ``jSrc``, ``iDst`` and
318+
``jDst`` are in the right range, and that ``src`` is a
319+
Matrix_complex_ball_dense with the same base ring as ``self``.
320+
321+
INPUT:
322+
323+
- ``iDst`` - the row to be copied to in ``self``.
324+
- ``jDst`` - the column to be copied to in ``self``.
325+
- ``src`` - the matrix to copy from. Should be a
326+
Matrix_complex_ball_dense with the same base ring as
327+
``self``.
328+
- ``iSrc`` - the row to be copied from in ``src``.
329+
- ``jSrc`` - the column to be copied from in ``src``.
330+
331+
TESTS::
332+
333+
sage: m = MatrixSpace(CBF,3,4)(range(12))
334+
sage: m
335+
[ 0 1.000000000000000 2.000000000000000 3.000000000000000]
336+
[4.000000000000000 5.000000000000000 6.000000000000000 7.000000000000000]
337+
[8.000000000000000 9.000000000000000 10.00000000000000 11.00000000000000]
338+
sage: m.transpose()
339+
[ 0 4.000000000000000 8.000000000000000]
340+
[1.000000000000000 5.000000000000000 9.000000000000000]
341+
[2.000000000000000 6.000000000000000 10.00000000000000]
342+
[3.000000000000000 7.000000000000000 11.00000000000000]
343+
sage: m.matrix_from_rows([0,2])
344+
[ 0 1.000000000000000 2.000000000000000 3.000000000000000]
345+
[8.000000000000000 9.000000000000000 10.00000000000000 11.00000000000000]
346+
sage: m.matrix_from_columns([1,3])
347+
[1.000000000000000 3.000000000000000]
348+
[5.000000000000000 7.000000000000000]
349+
[9.000000000000000 11.00000000000000]
350+
sage: m.matrix_from_rows_and_columns([1,2],[0,3])
351+
[4.000000000000000 7.000000000000000]
352+
[8.000000000000000 11.00000000000000]
353+
"""
354+
cdef Matrix_complex_ball_dense _src = <Matrix_complex_ball_dense> src
355+
acb_set(acb_mat_entry(self.value, iDst, jDst), acb_mat_entry(_src.value, iSrc, jSrc))
356+
310357
cpdef _richcmp_(left, right, int op):
311358
r"""
312359
EXAMPLES::

src/sage/matrix/matrix_cyclo_dense.pyx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,53 @@ cdef class Matrix_cyclo_dense(Matrix_dense):
407407

408408
return x
409409

410+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc):
411+
"""
412+
Copy the (iSrc,jSrc)-th entry of ``src`` to the (iDst,jDst)-th entry
413+
``self``.
414+
415+
WARNING: As the name suggests, expect segfaults if iSrc,jSrc,iDst,jDst
416+
are out of bounds!! This is for internal use only. This method assumes
417+
``src`` is a Matrix_cyclo_dense with the same base ring as ``self``.
418+
419+
INPUT:
420+
421+
- ``iDst`` - the row to be copied to in ``self``.
422+
- ``jDst`` - the column to be copied to in ``self``.
423+
- ``src`` - the matrix to copy from. Should be a Matrix_cyclo_dense
424+
with the same base ring as ``self``.
425+
- ``iSrc`` - the row to be copied from in ``src``.
426+
- ``jSrc`` - the column to be copied from in ``src``.
427+
428+
TESTS::
429+
430+
sage: K.<z> = CyclotomicField(3)
431+
sage: M = matrix(K,3,4,[i + z/(i+1) for i in range(12)])
432+
sage: M
433+
[ z 1/2*z + 1 1/3*z + 2 1/4*z + 3]
434+
[ 1/5*z + 4 1/6*z + 5 1/7*z + 6 1/8*z + 7]
435+
[ 1/9*z + 8 1/10*z + 9 1/11*z + 10 1/12*z + 11]
436+
sage: M.transpose()
437+
[ z 1/5*z + 4 1/9*z + 8]
438+
[ 1/2*z + 1 1/6*z + 5 1/10*z + 9]
439+
[ 1/3*z + 2 1/7*z + 6 1/11*z + 10]
440+
[ 1/4*z + 3 1/8*z + 7 1/12*z + 11]
441+
sage: M.matrix_from_rows([0,2])
442+
[ z 1/2*z + 1 1/3*z + 2 1/4*z + 3]
443+
[ 1/9*z + 8 1/10*z + 9 1/11*z + 10 1/12*z + 11]
444+
sage: M.matrix_from_columns([1,3])
445+
[ 1/2*z + 1 1/4*z + 3]
446+
[ 1/6*z + 5 1/8*z + 7]
447+
[ 1/10*z + 9 1/12*z + 11]
448+
sage: M.matrix_from_rows_and_columns([1,2],[0,3])
449+
[ 1/5*z + 4 1/8*z + 7]
450+
[ 1/9*z + 8 1/12*z + 11]
451+
"""
452+
cdef Matrix_cyclo_dense _src = src
453+
cdef int a
454+
for a in range(self._degree):
455+
self._matrix.copy_from_unsafe(a, jDst + iDst*self._ncols, _src._matrix, a, jSrc + iSrc*_src._ncols)
456+
410457
cdef bint get_is_zero_unsafe(self, Py_ssize_t i, Py_ssize_t j) except -1:
411458
r"""
412459
Return 1 if the entry ``(i, j)`` is zero, otherwise 0.

src/sage/matrix/matrix_dense.pyx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ cdef class Matrix_dense(matrix.Matrix):
138138
cdef Py_ssize_t i, j
139139
for j from 0<= j < nc:
140140
for i from 0<= i < nr:
141-
trans.set_unsafe(j,i,self.get_unsafe(i,j))
141+
trans.copy_from_unsafe(j, i, self, i, j)
142142

143143
if self._subdivisions is not None:
144144
row_divs, col_divs = self.subdivisions()
@@ -183,7 +183,7 @@ cdef class Matrix_dense(matrix.Matrix):
183183
rj -= 1
184184
for i from 0 <= i < nr:
185185
ri -= 1
186-
atrans.set_unsafe(j, i, self.get_unsafe(ri, rj))
186+
atrans.copy_from_unsafe(j, i, self, ri, rj)
187187

188188
if self._subdivisions is not None:
189189
row_divs, col_divs = self.subdivisions()

src/sage/matrix/matrix_gap.pyx

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,46 @@ cdef class Matrix_gap(Matrix_dense):
204204
"""
205205
self._libgap[i,j] = x
206206

207+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc):
208+
r"""
209+
Copy the ``(iSrc, jSrc)`` entry of ``src`` into the ``(iDst, jDst)``
210+
entry of ``self``.
211+
212+
INPUT:
213+
214+
- ``iDst`` - the row to be copied to in ``self``.
215+
- ``jDst`` - the column to be copied to in ``self``.
216+
- ``src`` - the matrix to copy from. Should be a Matrix_gap with the
217+
same base ring as ``self``.
218+
- ``iSrc`` - the row to be copied from in ``src``.
219+
- ``jSrc`` - the column to be copied from in ``src``.
220+
221+
TESTS::
222+
223+
sage: M = MatrixSpace(ZZ, 3, 4, implementation='gap')(range(12))
224+
sage: M
225+
[ 0 1 2 3]
226+
[ 4 5 6 7]
227+
[ 8 9 10 11]
228+
sage: M.transpose()
229+
[ 0 4 8]
230+
[ 1 5 9]
231+
[ 2 6 10]
232+
[ 3 7 11]
233+
sage: M.matrix_from_rows([0,2])
234+
[ 0 1 2 3]
235+
[ 8 9 10 11]
236+
sage: M.matrix_from_columns([1,3])
237+
[ 1 3]
238+
[ 5 7]
239+
[ 9 11]
240+
sage: M.matrix_from_rows_and_columns([1,2],[0,3])
241+
[ 4 7]
242+
[ 8 11]
243+
"""
244+
cdef Matrix_gap _src = <Matrix_gap>src
245+
self._libgap[iDst,jDst] = _src._libgap[iSrc,jSrc]
246+
207247
cpdef _richcmp_(self, other, int op):
208248
r"""
209249
Compare ``self`` and ``right``.

src/sage/matrix/matrix_generic_dense.pyx

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,47 @@ cdef class Matrix_generic_dense(matrix_dense.Matrix_dense):
9999
cdef get_unsafe(self, Py_ssize_t i, Py_ssize_t j):
100100
return self._entries[i*self._ncols + j]
101101

102+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc):
103+
r"""
104+
Copy the ``(iSrc, jSrc)`` entry of ``src`` into the ``(iDst, jDst)``
105+
entry of ``self``.
106+
107+
INPUT:
108+
109+
- ``iDst`` - the row to be copied to in ``self``.
110+
- ``jDst`` - the column to be copied to in ``self``.
111+
- ``src`` - the matrix to copy from. Should be a Matrix_generic_dense
112+
with the same base ring as ``self``.
113+
- ``iSrc`` - the row to be copied from in ``src``.
114+
- ``jSrc`` - the column to be copied from in ``src``.
115+
116+
TESTS::
117+
118+
sage: K.<z> = GF(9)
119+
sage: m = matrix(K,3,4,[((i%9)//3)*z + i%3 for i in range(12)])
120+
sage: m
121+
[ 0 1 2 z]
122+
[ z + 1 z + 2 2*z 2*z + 1]
123+
[2*z + 2 0 1 2]
124+
sage: m.transpose()
125+
[ 0 z + 1 2*z + 2]
126+
[ 1 z + 2 0]
127+
[ 2 2*z 1]
128+
[ z 2*z + 1 2]
129+
sage: m.matrix_from_rows([0,2])
130+
[ 0 1 2 z]
131+
[2*z + 2 0 1 2]
132+
sage: m.matrix_from_columns([1,3])
133+
[ 1 z]
134+
[ z + 2 2*z + 1]
135+
[ 0 2]
136+
sage: m.matrix_from_rows_and_columns([1,2],[0,3])
137+
[ z + 1 2*z + 1]
138+
[2*z + 2 2]
139+
"""
140+
cdef Matrix_generic_dense _src = <Matrix_generic_dense>src
141+
self._entries[iDst*self._ncols + jDst] = _src._entries[iSrc*_src._ncols + jSrc]
142+
102143
def _reverse_unsafe(self):
103144
r"""
104145
TESTS::

src/sage/matrix/matrix_generic_sparse.pyx

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,50 @@ cdef class Matrix_generic_sparse(matrix_sparse.Matrix_sparse):
200200
cdef get_unsafe(self, Py_ssize_t i, Py_ssize_t j):
201201
return self._entries.get((i,j), self._zero)
202202

203+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc):
204+
r"""
205+
Copy the ``(iSrc, jSrc)`` entry of ``src`` into the ``(iDst, jDst)``
206+
entry of ``self``.
207+
208+
INPUT:
209+
210+
- ``iDst`` - the row to be copied to in ``self``.
211+
- ``jDst`` - the column to be copied to in ``self``.
212+
- ``src`` - the matrix to copy from. Should be a Matrix_generic_sparse
213+
with the same base ring as ``self``.
214+
- ``iSrc`` - the row to be copied from in ``src``.
215+
- ``jSrc`` - the column to be copied from in ``src``.
216+
217+
TESTS::
218+
219+
sage: K.<z> = GF(9)
220+
sage: m = matrix(K,3,4,[((i%9)//3)*z + i%3 if is_prime(i) else 0 for i in range(12)],sparse=True)
221+
sage: m
222+
[ 0 0 2 z]
223+
[ 0 z + 2 0 2*z + 1]
224+
[ 0 0 0 2]
225+
sage: m.transpose()
226+
[ 0 0 0]
227+
[ 0 z + 2 0]
228+
[ 2 0 0]
229+
[ z 2*z + 1 2]
230+
sage: m.matrix_from_rows([0,2])
231+
[0 0 2 z]
232+
[0 0 0 2]
233+
sage: m.matrix_from_columns([1,3])
234+
[ 0 z]
235+
[ z + 2 2*z + 1]
236+
[ 0 2]
237+
sage: m.matrix_from_rows_and_columns([1,2],[0,3])
238+
[ 0 2*z + 1]
239+
[ 0 2]
240+
"""
241+
cdef Matrix_generic_sparse _src = <Matrix_generic_sparse>src
242+
if (iSrc,jSrc) in _src._entries:
243+
self._entries[(iDst,jDst)] = _src._entries.get((iSrc, jSrc), _src._zero)
244+
elif (iDst,jDst) in self._entries:
245+
del self._entries[(iDst,jDst)]
246+
203247
cdef bint get_is_zero_unsafe(self, Py_ssize_t i, Py_ssize_t j) except -1:
204248
"""
205249
Return 1 if the entry ``(i, j)`` is zero, otherwise 0.

src/sage/matrix/matrix_gf2e_dense.pyx

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,47 @@ cdef class Matrix_gf2e_dense(matrix_dense.Matrix_dense):
284284
cdef Cache_base cache = <Cache_base> self._base_ring._cache
285285
return cache.fetch_int(r)
286286

287+
cdef copy_from_unsafe(self, Py_ssize_t iDst, Py_ssize_t jDst, src, Py_ssize_t iSrc, Py_ssize_t jSrc):
288+
r"""
289+
Copy the ``(iSrc, jSrc)`` entry of ``src`` into the ``(iDst, jDst)``
290+
entry of ``self``.
291+
292+
INPUT:
293+
294+
- ``iDst`` - the row to be copied to in ``self``.
295+
- ``jDst`` - the column to be copied to in ``self``.
296+
- ``src`` - the matrix to copy from. Should be a Matrix_gf2e_dense with
297+
the same base ring as ``self``.
298+
- ``iSrc`` - the row to be copied from in ``src``.
299+
- ``jSrc`` - the column to be copied from in ``src``.
300+
301+
TESTS::
302+
303+
sage: K.<z> = GF(512)
304+
sage: m = matrix(K,3,4,[sum([(i//(2^j))%2 * z^j for j in range(4)]) for i in range(12)])
305+
sage: m
306+
[ 0 1 z z + 1]
307+
[ z^2 z^2 + 1 z^2 + z z^2 + z + 1]
308+
[ z^3 z^3 + 1 z^3 + z z^3 + z + 1]
309+
sage: m.transpose()
310+
[ 0 z^2 z^3]
311+
[ 1 z^2 + 1 z^3 + 1]
312+
[ z z^2 + z z^3 + z]
313+
[ z + 1 z^2 + z + 1 z^3 + z + 1]
314+
sage: m.matrix_from_rows([0,2])
315+
[ 0 1 z z + 1]
316+
[ z^3 z^3 + 1 z^3 + z z^3 + z + 1]
317+
sage: m.matrix_from_columns([1,3])
318+
[ 1 z + 1]
319+
[ z^2 + 1 z^2 + z + 1]
320+
[ z^3 + 1 z^3 + z + 1]
321+
sage: m.matrix_from_rows_and_columns([1,2],[0,3])
322+
[ z^2 z^2 + z + 1]
323+
[ z^3 z^3 + z + 1]
324+
"""
325+
cdef Matrix_gf2e_dense _src = <Matrix_gf2e_dense>src
326+
mzed_write_elem(self._entries, iDst, jDst, mzed_read_elem(_src._entries, iSrc, jSrc))
327+
287328
cdef bint get_is_zero_unsafe(self, Py_ssize_t i, Py_ssize_t j) except -1:
288329
r"""
289330
Return 1 if the entry ``(i, j)`` is zero, otherwise 0.

0 commit comments

Comments
 (0)