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:  sage6.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 builtin 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
 Branch set to u/kartikv/19081_flint_hnf
 Commit set to 4165319ff7ac640e7d1eaddf4345f902244ebaad
 Status changed from new to needs_review
comment:2 Changed 5 years ago by
 Commit changed from 4165319ff7ac640e7d1eaddf4345f902244ebaad to f11bdf34242077803401b3e2aaaac6cd6ee53da0
Branch pushed to git repo; I updated commit sha1. New commits:
f11bdf3  now with correctly sized transformation matrices

comment:3 Changed 5 years ago by
 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
 Commit changed from f11bdf34242077803401b3e2aaaac6cd6ee53da0 to e7fe591ceb793fbc2f686db836814703381cb3d6
Branch pushed to git repo; I updated commit sha1. New commits:
e7fe591  corrected comment

comment:5 Changed 5 years ago by
 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
 Status changed from needs_review to positive_review
looks good to me
comment:7 Changed 5 years ago by
 Branch changed from u/kartikv/19081_flint_hnf to e7fe591ceb793fbc2f686db836814703381cb3d6
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
adding flint as an option for HNF and using it as the default for moderately large matrices