Skip to content

Commit d5d101e

Browse files
author
Release Manager
committed
gh-39204: Speed up multimodular algorithm in bad case **Edit**: Now flint has fixed flintlib/flint#2129 , it should be better to just switch to flint entirely — according to flintlib/flint#2129 (comment) , one of the possible algorithms by flint is multimodular, which should be faster than or equal to what we're having now. If it is slower in any case, bug can be reported upstream. ------ Related to #39197. Technically the algorithm doesn't deviate from @williamstein 's original book; however the original book doesn't say *how many* additional primes to add each time. The original implementation roughly consider 3 more primes each time. This can be highly inefficient when there are more columns than rows, which makes the result's height much higher than the guess. This increases the length of `M` by roughly a factor of `1.2` each time. Worst case it makes the algorithm slower by a (hopefully small) constant factor. For the added test case, it appears to improve the performance. (Originally takes 40s, now takes <10s on my machine) ### 📝 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. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - #12345: short description why this is a dependency --> <!-- - #34567: ... --> URL: #39204 Reported by: user202729 Reviewer(s): Travis Scrimshaw
2 parents 32cf547 + ce55b7c commit d5d101e

File tree

1 file changed

+44
-4
lines changed

1 file changed

+44
-4
lines changed

src/sage/matrix/misc.pyx

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,44 @@ def matrix_rational_echelon_form_multimodular(Matrix self, height_guess=None, pr
204204
[ 0 0 1 2]
205205
sage: A.pivots()
206206
(0, 1, 2)
207+
208+
A small benchmark, showing that the multimodular algorithm is sometimes faster
209+
and sometimes slower than the flint algorithm::
210+
211+
sage: import copy
212+
sage: def benchmark(num_row, num_col, entry_size, timeout=2, integer_coefficient=True):
213+
....: A = matrix(QQ, [[
214+
....: randint(1, 2^entry_size) if integer_coefficient else ZZ(randint(1, 2^entry_size))/randint(1, 2^entry_size)
215+
....: for col in range(num_col)] for row in range(num_row)])
216+
....: data=[]
217+
....: for algorithm in ("flint", "padic", "multimodular"):
218+
....: # classical is too slow
219+
....: B = copy.copy(A)
220+
....: t = walltime()
221+
....: alarm(timeout)
222+
....: try:
223+
....: B.echelonize(algorithm=algorithm)
224+
....: except AlarmInterrupt:
225+
....: pass
226+
....: finally:
227+
....: cancel_alarm()
228+
....: data.append((round(walltime(t), 4), algorithm))
229+
....: return sorted(data)
230+
sage: benchmark(20, 20, 10000) # long time (multimodular wins)
231+
[...'multimodular'...'flint'...]
232+
sage: benchmark(39, 40, 200) # long time (flint wins)
233+
[...'flint'...'multimodular'...]
234+
235+
In this case, there are more columns than rows, which means the resulting
236+
matrix has height much higher than the input matrix. We check that the function
237+
does not take too long::
238+
239+
sage: A = matrix(QQ, [[randint(1, 2^500) for col in range(40)] for row in range(20)])
240+
sage: t = walltime()
241+
sage: A.echelonize(algorithm="multimodular") # long time
242+
sage: t = walltime(t) # long time
243+
sage: (t < 10, t) # long time
244+
(True, ...)
207245
"""
208246
if proof is None:
209247
from sage.structure.proof.proof import get_flag
@@ -221,10 +259,12 @@ def matrix_rational_echelon_form_multimodular(Matrix self, height_guess=None, pr
221259
height_guess = 10000000*(height+100)
222260
tm = verbose("height_guess = %s" % height_guess, level=2, caller_name="multimod echelon")
223261

262+
cdef Integer M
263+
from sage.arith.misc import integer_floor as floor
224264
if proof:
225-
M = self._ncols * height_guess * height + 1
265+
M = floor(max(1, self._ncols * height_guess * height + 1))
226266
else:
227-
M = height_guess + 1
267+
M = floor(max(1, height_guess + 1))
228268

229269
if self.is_sparse():
230270
from sage.matrix.matrix_modn_sparse import MAX_MODULUS
@@ -309,7 +349,7 @@ def matrix_rational_echelon_form_multimodular(Matrix self, height_guess=None, pr
309349
except ValueError as msg:
310350
verbose(msg, level=2)
311351
verbose("Not enough primes to do CRT lift; redoing with several more primes.", level=2, caller_name="multimod echelon")
312-
M = prod * p*p*p
352+
M <<= M.bit_length() // 5 + 1
313353
continue
314354

315355
if not proof:
@@ -321,7 +361,7 @@ def matrix_rational_echelon_form_multimodular(Matrix self, height_guess=None, pr
321361
verbose("Validity of result checked.", level=2, caller_name="multimod echelon")
322362
break
323363
verbose("Validity failed; trying again with more primes.", level=2, caller_name="multimod echelon")
324-
M = prod * p*p*p
364+
M <<= M.bit_length() // 5 + 1
325365
#end while
326366
verbose("total time",tm, level=2, caller_name="multimod echelon")
327367
return E, tuple(best_pivots)

0 commit comments

Comments
 (0)