Opened 7 years ago

Closed 7 years ago

#14032 closed defect (fixed)

determinant() of integer matrices of size in [51,63] broken

Reported by: jdemeyer Owned by: jason, was
Priority: critical Milestone: sage-5.7
Component: linear algebra Keywords:
Cc: Merged in: sage-5.7.beta2
Authors: Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

sage: random_matrix(ZZ,60).det()
[...]
RuntimeError: maximum recursion depth exceeded while calling a Python object

The problem was an infinite recursion where we compute a determinant over ZZ by working mod p and we compute a determinant over GF(p) by lifting to ZZ...

This recursion got broken because the bound to use LinBox for matrices mod p changed in #12883. This patch fixes this bound.

It also does various clean-up of doctests and removes _det_4x4_unsafe() which only added an extra level of indirection (determinant() -> _det_4x4_unsafe() -> four_dim_det())

Attachments (1)

14032_multimodular_det.patch (10.3 KB) - added by jdemeyer 7 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by vbraun

50 is the threshold for defaulting to algorithm='padic'. The following works fine: random_matrix(ZZ,60).determinant(algorithm='pari')

comment:2 Changed 7 years ago by jdemeyer

(never mind, forgot about caching)

Last edited 7 years ago by jdemeyer (previous) (diff)

comment:3 Changed 7 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from determinant() of large integer matrices broken to determinant() of integer matrices of size in [51,63] broken

comment:4 Changed 7 years ago by jdemeyer

There is an infinite recursion going on where we compute a determinant over ZZ by working mod p and we compute a determinant over GF(p) by lifting to ZZ...

comment:5 Changed 7 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Description modified (diff)

Changed 7 years ago by jdemeyer

comment:6 Changed 7 years ago by jdemeyer

  • Status changed from new to needs_review

comment:7 Changed 7 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Looks great!

comment:8 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.7.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.