Opened 5 years ago

Closed 5 years ago

#19081 closed enhancement (fixed)

More reimplementing matrix_integer_dense using FLINT

Reported by: kartikv Owned by:
Priority: major Milestone: sage-6.9
Component: linear algebra Keywords: matrix, dense
Cc: vbraun Merged in:
Authors: Kartik Venkatram Reviewers: R. Andrew Ohana
Report Upstream: N/A Work issues:
Branch: e7fe591 (Commits) Commit: e7fe591ceb793fbc2f686db836814703381cb3d6
Dependencies: Stopgaps:

Description

Now with hermite normal forms. Added a new algorithm choice ("flint") to hermite form, and updated cutoffs to account for the fact that pari0 generally beats flint < 125 rows/cols and FLINT wins above that. pari1 and pari4 seem to always be much worse, as is ntl and the built-in padic method, but I only tested up to matrices of a few hundred entries. Also, flint does better for smaller matrices with larger entries, so that should be accounted for as well, but this is a reasonable first pass.

Change History (7)

comment:1 Changed 5 years ago by kartikv

  • Authors set to Kartik Venkatram
  • Branch set to u/kartikv/19081_flint_hnf
  • Commit set to 4165319ff7ac640e7d1eaddf4345f902244ebaad
  • Status changed from new to needs_review

New commits:

4165319adding flint as an option for HNF and using it as the default for moderately large matrices

comment:2 Changed 5 years ago by git

  • Commit changed from 4165319ff7ac640e7d1eaddf4345f902244ebaad to f11bdf34242077803401b3e2aaaac6cd6ee53da0

Branch pushed to git repo; I updated commit sha1. New commits:

f11bdf3now with correctly sized transformation matrices

comment:3 Changed 5 years ago by ohanar

  • Cc vbraun added
  • Reviewers changed from vbraun to R. Andrew Ohana
  • Status changed from needs_review to needs_work

The description of the default algorithm doesn't match the code.

comment:4 Changed 5 years ago by git

  • Commit changed from f11bdf34242077803401b3e2aaaac6cd6ee53da0 to e7fe591ceb793fbc2f686db836814703381cb3d6

Branch pushed to git repo; I updated commit sha1. New commits:

e7fe591corrected comment

comment:5 Changed 5 years ago by kartikv

  • Status changed from needs_work to needs_review

Sorry about that, too much undoing. Should be right now.

comment:6 Changed 5 years ago by ohanar

  • Status changed from needs_review to positive_review

looks good to me

comment:7 Changed 5 years ago by vbraun

  • Branch changed from u/kartikv/19081_flint_hnf to e7fe591ceb793fbc2f686db836814703381cb3d6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.