update M4RI to newest upstream release #9475

malb · 2010-07-11T16:12:22Z

The new version improves elimination to some extent, comes with a cleaner API and has an option to suppress SSE instructions.

Note to the release managers

Apply only m4ri_new_version.v2.patch to the Sage library when merging the new M4RI spkg.

Upstream: Not yet reported upstream; Will do shortly.

CC: @sagetrac-mariah

Component: packages: standard

Keywords: M4RI, spkg-check

Author: Martin Albrecht, Leif Leonhardy

Reviewer: Leif Leonhardy, David Kirkby, Mariah Lenox

Merged: sage-4.5.3.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/9475

malb · 2010-07-11T16:14:02Z

comment:1

I've uploaded an SPKG to

http://sage.math.washington.edu/home/malb/spkgs/libm4ri-20100701.spkg

This SPKG also takes care of #9381 (SAGE_FAT_BINARY not being respected) and the M4RI part of #9281 (spkg-check)

malb · 2010-07-11T20:17:42Z

comment:2

I've run the M4RI self test (not the Sage test suite) on the following machines:

x86_64 Linux (Xeon, sage.math and eno);
x86 OSX (Xeon, bsd);
ia64 Linux (iras);
UltraSPARC T2 Solaris using GCC (t2.math.washington.edu)
x86 Linux (VirtualBox);

sagetrac-mariah · 2010-07-13T15:53:13Z

comment:3

SPKG.txt under "Releases" says latest
release is libm4ri-20100107, do you mean
libm4ri-20100701?
Since spkg-check exists, in spkg-install
the commented out lines:

# $MAKE check
# if [ $? -ne 0 ]; then
#     echo "libm4ri testsuite failed, please report upstream!"
#     exit 1
# fi

can be removed.

In spkg-install, if SAGE_FAT_BINARY is yes, then
sse2 is disabled. What about sse3 as was the reported
problem in #9381
This version of libm4ri does not appear under the
downloads on the m4ri web site,
thus no way to tell if the source in the spkg corresponds
to the source under the claimed version.

malb · 2010-07-13T18:22:28Z

comment:5

Replying to @sagetrac-mariah:

SPKG.txt under "Releases" says latest release is libm4ri-20100107, do you mean libm4ri-20100701?

Fixed.

Since spkg-check exists, in spkg-install the commented out lines: # $MAKE check # if [ $? -ne 0 ]; then # echo "libm4ri testsuite failed, please report upstream!" # exit 1 # fi can be removed.

Fixed.

In spkg-install, if SAGE_FAT_BINARY is yes, then sse2 is disabled. What about sse3 as was the reported problem in #9381

We never use SSE3, but yeah all checks for SSEx instructions are suppressed in that case.

This version of libm4ri does not appear under the downloads on the m4ri web site, thus no way to tell if the source in the spkg corresponds to the source under the claimed version.

This is because I am upstream and wanted to wait for portability issues before putting the release on the website. I'm putting it online now, if that makes your life easier.

malb · 2010-07-13T18:41:20Z

comment:6

See

http://m4ri.sagemath.org

and

http://bitbucket.org/malb/m4ri/wiki/M4RI-20100701

for the new upstream release.

nexttime · 2010-07-14T13:18:40Z

comment:7

The changes to SPKG.txt, spkg-install and spkg-check are not checked in.

nexttime · 2010-07-14T14:00:54Z

comment:8

SPKG.txt:

Author should be Authors
Is the given e-mail address still up-to-date?
I'd substitute "function names match what the function is doing now"
by "function names now match ... is doing"
s/supress/suppress/
s/SSE2 instruction/SSE2 instructions/

spkg-install:

Old typo, I guess: s/CLFAGS/CFLAGS/ twice
The choice of the variable name SSE2_SUPPORT isn't that nice; I'd rather call it DISABLE_SSE2, but never mind.

spkg-check:

Uses make rather than $MAKE (in contrast to spkg-install), but shouldn't be a problem.

nexttime · 2010-07-14T15:49:57Z

comment:9

I'd suggest adding the (standard) section Special Update/Build Instructions to SPKG.txt, with the following:

Remove upstream Mercurial repository (directory src/.hg, file src/.hgtags)
Remove directory src/autom4te.cache, file src/m4ri.vcproj

I've built a p1 spkg with the above mentioned files removed: 392KB vs. 1.2MB

If there are no dependencies on Sage packages (which is the case here), we don't need to add $SAGE_LOCAL/include to the preprocessor search path in spkg-install (CFLAGS and CPPFLAGS).

Perhaps touch src/configure, because it has the same time stamp as configure.*.

Upstream notes:

There is no target clean.
src/NEWS and src/ChangeLog are empty files. ;-)

Tests in progress,

Leif

nexttime · 2010-07-14T17:50:56Z

comment:10

Ubuntu 9.04 x86 (P4 Prescott, gcc 4.3.3) Sage 4.5.rc0 (with ECL 10.2.1.p1):

ptestlong: All tests passed.
- Ubuntu 9.04 x86_64 (Core2, gcc 4.3.3) Sage 4.5.rc0:
ptestlong: All tests passed.
(with my stripped-down M4RI spkg, see above)

I've only installed the new package (dated July 13th) and applied the patch, i.e. did not build Sage from scratch, and haven't (yet) run the testsuite.

nexttime · 2010-07-14T18:53:19Z

comment:11

Shame on me I missed that one:

./spkg-install: line 47: [x: command not found

if ["x$SAGE_FAT_BINARY" = "xyes"]; then
    SSE2_SUPPORT="--disable-sse2"
else
    SSE2_SUPPORT=""
fi

should be

if [ "x$SAGE_FAT_BINARY" = "xyes" ]; then
    ...

Note that [ is actually a command (namely an alias for or link to test, depending on the shell), and ] is its last parameter.

On both of the above systems, the testsuite passed without errors.

(I reinstalled the package(s) with SAGE_CHECK="yes".)

nexttime · 2010-07-14T19:35:43Z

comment:12

Just let me know if I should provide (a) reviewer patch(es) to the spkg (after a commit to the changes not yet checked in); I cannot upload an updated spkg to sage.math though.

Leif

malb · 2010-07-14T20:39:25Z

comment:13

Replying to @nexttime:

Remove upstream Mercurial repository (directory src/.hg, file src/.hgtags)

Remove directory src/autom4te.cache, file src/m4ri.vcproj
I've built a p1 spkg with the above mentioned files removed: 392KB vs. 1.2MB

It's strange that there was an autom4te.cache, since I rm it in my release script. I'm okay with these changes.

malb · 2010-07-14T20:41:14Z

comment:14

Leif,

I'd appreciate if you could update SPKG according to your suggestions, I'm okay with them all. If you upload the SPKG somewhere or send it my way I can upload it to sage.math.

nexttime · 2010-07-14T21:00:56Z

comment:15

Replying to @malb:

I'd appreciate if you could update SPKG according to your suggestions, I'm okay with them all. If you upload the SPKG somewhere or send it my way I can upload it to sage.math.

Ok, I'll create a cumulative spkg patch and a "stripped-down" p1 package in a few hours and then mail you both.

(I assume your .ac.uk e-mail address in SPKG.txt is still appropriate as upstream contact.)

Currently running stress-test builds of 4.5.rc1... ;-)

Leif

malb · 2010-07-14T21:22:01Z

comment:16

Yes, that's still current, however martinralbrecht@ googleblablabla might be current for a longer time.

Thanks!

nexttime · 2010-07-16T02:12:07Z

Attachment: trac_9475-libm4ri-20100701.p1-based_on_p0.patch.gz

You have to commit Martin's changes ("p0") first to apply this.

nexttime · 2010-07-16T02:33:07Z

comment:17

Patch for p1 Sage package is up. Remember to commit Martin's changes (of July 13th) first before you apply this patch. (This patch does not remove the unnecessary files in src/ since they are not under our version control; they are of course deleted from the p1 spkg.)

(Link to) New libm4ri-20100701.p1.spkg is on the way.

Tested with both 4.5.rc0 and rc1 (Ubuntu 9.04 x86_64).

Sorry for the delay.

-Leif

nexttime · 2010-08-07T07:24:16Z

comment:33

Replying to @qed777:

Can someone put the ticket number in the commit string?

Done, but now we have yet another attachment since I couldn't replace Martin's.

(I would have thought his patch's comment was sufficient to conclude that the patch has to be applied to the Sage library repository... ;-) )

qed777 · 2010-08-09T23:00:04Z

comment:34

Note: I haven't merged this ticket into 4.5.3.alpha0, because I noticed some segfaults that appear to stem from the new package and/or patch, when I doctested various trial alpha0s on sage.math. At the moment, it seems best to put out a 4.5.3.alpha0 with passing doctests and base on this any necessary efforts to merge the new M4RI into alpha1.

qed777 · 2010-08-10T04:18:41Z

comment:35

Of course, all long tests now pass (well, there are no reproducible failures) on sage.math with 4.5.3.alpha0 + #9475, so it seems no new efforts are necessary. I'm checking bsd, redhawk, and t2 now.

qed777 · 2010-08-10T05:47:06Z

comment:36

Replying to @qed777:

Of course, all long tests now pass (well, there are no reproducible failures) on sage.math with 4.5.3.alpha0 + #9475, so it seems no new efforts are necessary. I'm checking bsd, redhawk, and t2 now.

The long doctests also pass on bsd, redhawk, and t2.

malb · 2010-08-10T14:02:05Z

comment:37

There definitely is a bug in this new M4RI, I do get SIGSEGVs on my new laptop. I'll investigate.

malb · 2010-08-10T14:42:54Z

comment:38

        while(it!=end){
            Exponent e=*it; 
                from_term_map_type::const_iterator from_it=eliminated2row_number.find(e);
                assert(terms_as_exp_step1[row_start[from_it->second]]==e);
                assert(from_it!=eliminated2row_number.end());
 ===>               int index=from_it->second;//...translate e->line number;
                mzd_write_bit(mat_step2_factor,i,index,1);
            it++;
        }

This is where pbori.pyx crashes for me. I installed a new GCC today, so maybe that's to blame?

malb · 2010-08-10T15:01:33Z

comment:39

I think I got it: TheTrac macro PolyBoRi SPKG in 4.5.2 ships its own M4RI (and Boost) which conflicts with this new M4RI SPKG, thus since Leif removed this redundant copy of M4RI in the PolyBoRi, it works in 4.5.3.alpha0 but not before.

nexttime · 2010-08-10T15:14:53Z

comment:40

Replying to @malb:

I think I got it: The PolyBoRi SPKG in 4.5.2 ships its own M4RI (and Boost) which conflicts with this new M4RI SPKG, thus since Leif removed this redundant copy of M4RI in the PolyBoRi, it works in 4.5.3.alpha0 but not before.

LOL! (Sometimes little clean-ups make more sense than expected...)

malb · 2010-08-11T15:50:05Z

comment:41

It seems this ticket is incompatible with#9717. On my laptop I always get SIGSEGVs in pbori.pyx

malb · 2010-08-11T17:58:07Z

comment:42

I tracked down the issue. The cause is some assumptions inTrac macro PolyBoRi about M4RI which are not met anymore. This ticket can go in I say

malb · 2010-08-11T18:04:57Z

comment:43

Actually, this ticket can only go in if a fix forTrac macro PolyBoRi is also accepted, cf. #9717

jasongrout · 2010-08-12T04:34:15Z

comment:44

Minor thing: the documentation for rank still says:

On average 'lqup' should be faster than 'm4ri' and hence it is
the default choice.

malb · 2010-08-12T18:48:54Z

comment:45

#9717 has an updated, fixedTrac macro PolyBoRi SPKG

malb · 2010-08-12T18:51:34Z

comment:46

The updated patch only replaces the mention of LQUP with PLS

qed777 · 2010-08-12T20:15:41Z

comment:47

Should someone review the latest changes?

malb · 2010-08-12T20:48:50Z

comment:48

The only change in the new version compared to the previous version of m4ri_new_version.v2.patch is that one mention of LQUP was replaced by PLS in a docstring. That's all, this is why I didn't reset the status.

nexttime · 2010-08-13T03:05:08Z

comment:49

Replying to @malb:

The only change in the new version compared to the previous version of m4ri_new_version.v2.patch is that one mention of LQUP was replaced by PLS in a docstring. That's all, this is why I didn't reset the status.

Not 100%:

--- m4ri_new_version.v2.patch.orig	2010-08-07 09:00:21.000000000 +0200
+++ m4ri_new_version.v2.patch	2010-08-12 20:50:56.000000000 +0200
@@ -1,14 +1,14 @@
 # HG changeset patch
 # User Martin Albrecht <[email protected]>
 # Date 1277764034 -3600
-# Node ID 3365789479e6d70cb1930b2e97c7874cbd3310db
-# Parent  ba36200d8a2f844179785580245fd95aa6401a51
+# Node ID 3b116dd35a84e0b6bd8ea12a732b8fa1fbda796f
+# Parent  0bb69a98789215c64a81c4602f46a50c0aeca5f0
 #9475 Adapts Sage library interface to new M4RI API (libm4ri-20100701)
 
-diff -r ba36200d8a2f -r 3365789479e6 module_list.py
---- a/module_list.py	Fri Jun 25 10:05:59 2010 +0100
+diff -r 0bb69a987892 -r 3b116dd35a84 module_list.py
+--- a/module_list.py	Tue Aug 10 13:46:10 2010 +0100
 +++ b/module_list.py	Mon Jun 28 23:27:14 2010 +0100
-@@ -783,7 +783,7 @@
+@@ -807,7 +807,7 @@
      Extension('sage.matrix.matrix_mod2_dense',
                sources = ['sage/matrix/matrix_mod2_dense.pyx'],
                libraries = ['gmp','m4ri', 'gd', 'png12', 'z'],
@@ -17,7 +17,7 @@
  
      Extension('sage.matrix.matrix_modn_dense',
                sources = ['sage/matrix/matrix_modn_dense.pyx'],
-@@ -971,7 +971,7 @@
+@@ -995,7 +995,7 @@
      Extension('sage.modules.vector_mod2_dense',
                sources = ['sage/modules/vector_mod2_dense.pyx'],
                libraries = ['gmp','m4ri', 'png12', 'gd'],
@@ -26,8 +26,8 @@
      
      Extension('sage.modules.vector_rational_dense',
                sources = ['sage/modules/vector_rational_dense.pyx'],
-diff -r ba36200d8a2f -r 3365789479e6 sage/libs/m4ri.pxd
---- a/sage/libs/m4ri.pxd	Fri Jun 25 10:05:59 2010 +0100
+diff -r 0bb69a987892 -r 3b116dd35a84 sage/libs/m4ri.pxd
+--- a/sage/libs/m4ri.pxd	Tue Aug 10 13:46:10 2010 +0100
 +++ b/sage/libs/m4ri.pxd	Mon Jun 28 23:27:14 2010 +0100
 @@ -141,6 +141,9 @@
      # reduced row echelon form from upper triangular form
@@ -60,8 +60,8 @@
  
      # reduced row echelon form using PLUQ factorization
      cdef long mzd_echelonize_pluq(mzd_t *A, int full)
-diff -r ba36200d8a2f -r 3365789479e6 sage/matrix/matrix_mod2_dense.pyx
---- a/sage/matrix/matrix_mod2_dense.pyx	Fri Jun 25 10:05:59 2010 +0100
+diff -r 0bb69a987892 -r 3b116dd35a84 sage/matrix/matrix_mod2_dense.pyx
+--- a/sage/matrix/matrix_mod2_dense.pyx	Tue Aug 10 13:46:10 2010 +0100
 +++ b/sage/matrix/matrix_mod2_dense.pyx	Mon Jun 28 23:27:14 2010 +0100
 @@ -1010,15 +1010,16 @@
      #    * Matrix windows -- only if you need strassen for that base
@@ -116,7 +116,28 @@
                  k = 0
  
              _sig_on
-@@ -1681,7 +1691,7 @@
+@@ -1106,6 +1116,20 @@
+             self.cache('rank', r)
+             self.cache('pivots', self._pivots())
+ 
++        elif algorithm == 'top':
++            
++            self.check_mutability()
++            self.clear_cache()        
++
++            _sig_on
++            mzd_top_echelonize_m4ri(self._entries, 0)
++            r = 0
++            _sig_off
++            
++            self.cache('in_echelon_form',True)
++            self.cache('rank', r)
++            self.cache('pivots', self._pivots())
++
+         elif algorithm == 'linbox':
+ 
+             #self._echelonize_linbox()
+@@ -1681,7 +1705,7 @@
              sage: float(d)
              0.63184899999999999
              sage: A.density(approx=True)
@@ -125,7 +146,7 @@
              sage: float(len(A.nonzero_positions())/1000^2)
              0.63184899999999999
          """
-@@ -1691,7 +1701,7 @@
+@@ -1691,18 +1715,18 @@
          else:
              return matrix_dense.Matrix_dense.density(self)
  
@@ -134,7 +155,11 @@
          """
          Return the rank of this matrix.
  
-@@ -1702,7 +1712,7 @@
+-        On average 'lqup' should be faster than 'm4ri' and hence it is
++        On average 'pls' should be faster than 'm4ri' and hence it is
+         the default choice. However, for small - i.e. quite few
+         thousand rows & columns - and sparse matrices 'm4ri' might be
+         a better choice.
  
          INPUT:
  
@@ -143,7 +168,7 @@
  
          EXAMPLE::
  
-@@ -1722,10 +1732,10 @@
+@@ -1722,10 +1746,10 @@
          cdef mzd_t *A = mzd_copy(NULL, self._entries)
          cdef mzp_t *P, *Q
  
@@ -156,7 +181,7 @@
              mzp_free(P)
              mzp_free(Q)
          elif algorithm == 'm4ri':
-@@ -2060,9 +2070,9 @@
+@@ -2060,9 +2084,9 @@
      mzp_free(q)
      return B,P,Q
  
@@ -168,7 +193,7 @@
  
      INPUT:
          A -- matrix
-@@ -2074,14 +2084,14 @@
+@@ -2074,14 +2098,14 @@
  
      EXAMPLE::
  
@@ -185,7 +210,7 @@
          sage: LU
          [1 0 0 1]
          [1 1 0 0]
-@@ -2095,7 +2105,7 @@
+@@ -2095,7 +2119,7 @@
          [0, 1, 2, 3]
  
          sage: A = random_matrix(GF(2),1000,1000)
@@ -194,7 +219,7 @@
          True
      """
      cdef Matrix_mod2_dense B = A.__copy__()
-@@ -2104,15 +2114,15 @@
+@@ -2104,15 +2128,15 @@
  
      if algorithm == 'standard':
          _sig_on

malb · 2010-08-13T08:44:06Z

comment:50

Argh, I'm an idiot! I'll update the patch, the 'top' stuff must be removed

malb · 2010-08-13T08:44:21Z

Sage library patch - needed to comply with new M4RI API (libm4ri-20100701). (Contains ticket number; apply only this one.)

nexttime · 2010-08-13T09:43:20Z

comment:51

Attachment: m4ri_new_version.v2.patch.gz

Replying to @malb:

Argh, I'm an idiot! I'll update the patch, the 'top' stuff must be removed

Nevertheless, passed all long tests with Sage 4.5.3.alpha0 and PolyBoRi 0.6.4.p4 from #9717 on Fedora 13 x86 (Pentium 4 Prescott, gcc 4.4.4).

The newly uploaded patch contains only the desired changes.

qed777 · 2010-08-15T08:03:14Z

Merged: sage-4.5.3.alpha0

qed777 · 2010-08-15T08:27:27Z

Changed merged from sage-4.5.3.alpha0 to sage-4.5.3.alpha1

malb added this to the sage-4.5.2 milestone Jul 11, 2010

malb added c: packages: standard labels Jul 11, 2010

malb added the s: needs review label Jul 11, 2010

sagetrac-mariah mannequin added s: needs work and removed s: needs review labels Jul 13, 2010

malb added s: needs review and removed s: needs work labels Jul 13, 2010

nexttime mannequin added s: needs work and removed s: needs review labels Jul 14, 2010

This comment has been minimized.

Sign in to view

nexttime mannequin removed the s: needs work label Jul 16, 2010

nexttime mannequin added the s: positive review label Aug 7, 2010

malb added s: needs work and removed s: positive review labels Aug 10, 2010

malb added s: positive review and removed s: needs work labels Aug 10, 2010

qed777 mannequin removed the s: positive review label Aug 15, 2010

qed777 mannequin closed this as completed Aug 15, 2010

sagetrac-drkirkby mannequin mentioned this issue Apr 1, 2020

METATICKET - missing spkg-check files / OpenSolaris & Solaris 10 test results. #9281

Closed

update M4RI to newest upstream release #9475

update M4RI to newest upstream release #9475

Comments

malb commented Jul 11, 2010

Note to the release managers

malb commented Jul 11, 2010

malb commented Jul 11, 2010

sagetrac-mariah mannequin commented Jul 13, 2010

malb commented Jul 13, 2010

malb commented Jul 13, 2010

nexttime mannequin commented Jul 14, 2010

nexttime mannequin commented Jul 14, 2010

nexttime mannequin commented Jul 14, 2010

nexttime mannequin commented Jul 14, 2010

nexttime mannequin commented Jul 14, 2010

nexttime mannequin commented Jul 14, 2010

malb commented Jul 14, 2010

malb commented Jul 14, 2010

nexttime mannequin commented Jul 14, 2010

malb commented Jul 14, 2010

nexttime mannequin commented Jul 16, 2010

nexttime mannequin commented Jul 16, 2010

This comment has been minimized.

nexttime mannequin commented Aug 7, 2010

qed777 mannequin commented Aug 9, 2010

qed777 mannequin commented Aug 10, 2010

qed777 mannequin commented Aug 10, 2010

malb commented Aug 10, 2010

malb commented Aug 10, 2010

malb commented Aug 10, 2010

nexttime mannequin commented Aug 10, 2010

malb commented Aug 11, 2010

malb commented Aug 11, 2010

malb commented Aug 11, 2010

jasongrout commented Aug 12, 2010

malb commented Aug 12, 2010

malb commented Aug 12, 2010

qed777 mannequin commented Aug 12, 2010

malb commented Aug 12, 2010

nexttime mannequin commented Aug 13, 2010

malb commented Aug 13, 2010

malb commented Aug 13, 2010

nexttime mannequin commented Aug 13, 2010

qed777 mannequin commented Aug 15, 2010

qed777 mannequin commented Aug 15, 2010