Thanks for the suggestions. In my application (and perhaps in many others) I do not care what order the diagonal entries are in (pari's convention is that the largest is top left, while most others put the smallest there). So I am working directly with the pari matrices and bypassing this function altogether,
As well as offering FLINT as an alternative algorithm (+1) we should also allow the user to choose the ordering of the diagonal entries, with current behaviour as the default. John On Thu, 11 Oct 2018 at 07:05, Vincent Delecroix <20100.delecr...@gmail.com> wrote: > Indeed, this is a terribly naive code to permute the entries... assuming > that the following is fast > > U,V,D = m.__pari__().matsnf(1).sage() > > permuting the entries should be done > > - in place > - not in Python > > I would also suggest to provide access to the alternative implementation > with flint > > void fmpz_mat_snf(fmpz_mat_t S, const fmpz_mat_t A); > > Best > Vincent > > Le 11/10/2018 à 01:20, William Stein a écrit : > > On Wed, Oct 10, 2018 at 3:29 PM Travis Scrimshaw <tsc...@ucdavis.edu> > wrote: > >> > >> So the bulk of the time is spent converting the Pari object back into a > Sage object; mainly, this line: > >> > >> U = self.matrix_space(ncols = self._nrows)([v[0][i,j] for i in > xrange(self._nrows-1,-1,-1) for j in xrange(self._nrows)]) > >> > >> which has 1000x1000 calls to extra the data and then has to be > reconstituted into a matrix object. I am not sure if there is a way around > this. It might just be a by-product of outsourcing the computation. > >> > > > > Thanks for boiling this down to the bottleneck. It can probably be > > made much faster. Instead of doing > > > > v[0][i,j], > > > > which probably involves at least two complicated pari calls, is there > > a way to convert v[0] to a Python list in one call? If not, one could > > add an optimized way to do so. Then convert that list to a matrix in > > sage. > > > >> Best, > >> Travis > >> > >> > >> On Thursday, October 11, 2018 at 5:58:04 AM UTC+10, John Cremona wrote: > >>> > >>> The method smith_form() for matrices over ZZ uses pari's matsnf() but > is vastly slower. Compare these: > >>> > >>> sage: M=MatrixSpace(ZZ,1000,5).random_element() > >>> sage: %timeit S=M.smith_form(transformation=True) > >>> 1 loop, best of 3: 2.57 s per loop > >>> sage: pM=pari(M) > >>> sage: %timeit S=pM.matsnf(1) > >>> 10 loops, best of 3: 49.5 ms per loop > >>> > >>> That's a factor of 50. There's something to be done in the interface > because pari's convention is opposite to Sage's so one has to reverse the > order of rows/columns in the output from pari, but still... > >>> > >>> John > >> > >> -- > >> You received this message because you are subscribed to the Google > Groups "sage-devel" group. > >> To unsubscribe from this group and stop receiving emails from it, send > an email to sage-devel+unsubscr...@googlegroups.com. > >> To post to this group, send email to sage-devel@googlegroups.com. > >> Visit this group at https://groups.google.com/group/sage-devel. > >> For more options, visit https://groups.google.com/d/optout. > > > > > > > > -- > You received this message because you are subscribed to the Google Groups > "sage-devel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to sage-devel+unsubscr...@googlegroups.com. > To post to this group, send email to sage-devel@googlegroups.com. > Visit this group at https://groups.google.com/group/sage-devel. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.