Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update M4RI to newest upstream release #12840

Closed
malb opened this issue Apr 13, 2012 · 63 comments
Closed

update M4RI to newest upstream release #12840

malb opened this issue Apr 13, 2012 · 63 comments

Comments

@malb
Copy link
Member

malb commented Apr 13, 2012

As the title says.

**Install: **http://sage.math.washington.edu/home/malb/spkgs/libm4ri-20120613.spkg

Apply: attachment: trac_12840_m4ri_new_version.patch, attachment: trac_12840-rebased-on-13109.patch

Apply to SAGE_ROOT: attachment: trac_12840_m4ri_png.patch

Depends on #12841
Depends on #13109

Dependencies: to be merged with #12841, #13109

Component: packages: standard

Author: Martin Albrecht, John Palmieri

Reviewer: Simon King, Volker Braun

Merged: sage-5.3.beta0

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

@malb malb added this to the sage-5.1 milestone Apr 13, 2012
@malb

This comment has been minimized.

@malb

This comment has been minimized.

@malb

This comment has been minimized.

@malb
Copy link
Member Author

malb commented Apr 13, 2012

Dependencies: #12841

@malb
Copy link
Member Author

malb commented Apr 14, 2012

comment:7

Attachment: trac_12840_m4ri_new_version.patch.gz

Updated the patch because I forgot to raise a ZeroDivisionError in __invert__. Fixed now.

@simon-king-jena
Copy link
Member

comment:8

After applying the patch and the spkg and rebuilding the polybori spkg, sage -br failed with

gcc -pthread -shared -L/mnt/local/king/SAGE/stable/sage-5.0.beta13/local/lib build/temp.linux-x86_64-2.7/sage/matrix/matrix_integer_dense.o -L/mnt/local/king/SAGE/stable/sage-5.0.beta13/local/lib -L/mnt/local/king/SAGE/stable/sage-5.0.beta13/local/lib -lcsage -liml -lpari -lm -lgmp -lcblas -latlas -lstdc++ -lntl -lpython2.7 -o build/lib.linux-x86_64-2.7/sage/matrix/matrix_integer_dense.so
error: command 'gcc' failed with exit status 1
Error installing modified sage library code.

No error message, I am afraid.

@simon-king-jena
Copy link
Member

comment:9

PS: That was with sage-5.0.beta13 plus a lot of patches, but none of them changing matrices.

@simon-king-jena
Copy link
Member

comment:10

And repeating sage -br, I finally got a proper error:

In file included from /mnt/local/king/SAGE/stable/sage-5.0.beta13/local/include/m4rie/m4rie.h:56,
                 from sage/matrix/matrix_mod2e_dense.cpp:240:
/mnt/local/king/SAGE/stable/sage-5.0.beta13/local/include/m4rie/permutation.h:30:30: error: m4ri/permutation.h: Datei oder Verzeichnis nicht gefunden
error: command 'gcc' failed with exit status 1
Error installing modified sage library code.

@simon-king-jena
Copy link
Member

comment:11

PS: permutation.h seems not to be present in the sources.

@simon-king-jena
Copy link
Member

comment:12

PS: The same error occurs when trying to rebuild libm4rie spkg: M4RIE expects permutation.h

@malb
Copy link
Member Author

malb commented May 3, 2012

comment:13

Yes, #12841 is a dependency of this ticket.

@simon-king-jena
Copy link
Member

comment:14

Ouch. I missed #12841. Sorry.

@simon-king-jena
Copy link
Member

comment:15

Am I right that both spkgs have to be installed, but #12840 goes before #12841?

@simon-king-jena
Copy link
Member

comment:16

Yippie! Applying the patches from #12840 and #12841, installing the new m4ri spkg from here, then installing the new m4rie spkg from #12841 (why is it on a different ticket? Can one live without the other?), re-installing polybori and finally sage -br, Sage starts!

@malb
Copy link
Member Author

malb commented May 3, 2012

comment:17

Replying to @simon-king-jena:

Yippie! Applying the patches from #12840 and #12841, installing the new m4ri spkg from here, then installing the new m4rie spkg from #12841 (why is it on a different ticket? Can one live without the other?),

No, you're right, they are co-dependent.

@simon-king-jena
Copy link
Member

comment:18

First of all: make ptestlong works with the two new spkgs (even though I have a few other patches applied...).

Here is a small timing.

Vanilla Sage-5.0.rc0:

sage: K = GF(2)
sage: %time A = random_matrix(K,20000,20000)
CPU times: user 0.29 s, sys: 0.02 s, total: 0.31 s
Wall time: 0.31 s
sage: %time B = random_matrix(K,20000,20000)
CPU times: user 0.23 s, sys: 0.03 s, total: 0.26 s
Wall time: 0.27 s
sage: %time (A*B).rank()
CPU times: user 11.70 s, sys: 0.05 s, total: 11.75 s
Wall time: 11.81 s
19937

Sage-5.0.beta13 with the new spkgs (and some other patches that shouldn't affect the timing for matrix multiplication):

sage: K = GF(2)
sage: %time A = random_matrix(K,20000,20000)
CPU times: user 0.30 s, sys: 0.03 s, total: 0.33 s
Wall time: 0.33 s
sage: %time B = random_matrix(K,20000,20000)
CPU times: user 0.32 s, sys: 0.01 s, total: 0.33 s
Wall time: 0.33 s
sage: %time (A*B).rank()
CPU times: user 11.70 s, sys: 0.10 s, total: 11.80 s
Wall time: 11.83 s
19937

Well, at least there is no slow-down...

Do you claim that the new version of M4RI is faster? You do claim a speed-up for M4RIE, on #12841.

@malb
Copy link
Member Author

malb commented May 3, 2012

comment:19

Replying to @simon-king-jena:

Do you claim that the new version of M4RI is faster? You do claim a speed-up for M4RIE, on #12841.

No, M4RI shouldn't have changed (at least the stuff, only bugfixes & refactoring.

That said, there's a new function for inverting upper triangular matrices which is much faster than the generic method, but it's not exposed yet. I need to implement the same for lower triangular matrices, then we can speed up inversions in general.

Furthermore, M4RI ships its own PNG reader/writer now which is much faster than what we have in Sage and uses much less memory. Again, it's not used yet. This patch just gets the new version in.

@simon-king-jena
Copy link
Member

Reviewer: Simon King

@simon-king-jena
Copy link
Member

comment:20

Since make ptestlong passes and the hg repository in the package is fine and SPKG.txt is up to date, I think I can give a positive review. However, I only tested on one platform. So, if the release manager believes that this is not enough, then please speak up!

@jdemeyer
Copy link

jdemeyer commented May 7, 2012

Changed dependencies from #12841 to to be merged wth #12841

@jdemeyer
Copy link

jdemeyer commented May 7, 2012

Changed dependencies from to be merged wth #12841 to to be merged with #12841

@jdemeyer
Copy link

jdemeyer commented May 8, 2012

comment:23

This fails to install on the Skynet machine "eno" (Fedora 16 x86_64):

/bin/sh ./libtool --tag=CC   --mode=link gcc -std=gnu99 -mmmx -msse -msse2 -msse3   -g -fPIC -Wall -pedantic -O2 -release 0.0.20111203 -no
-undefined  -o libm4ri.la -rpath /home/buildbot/build/sage/eno-1/eno_full/build/sage-5.1.beta1/local/lib brilliantrussian.lo misc.lo mzd.l
o graycode.lo strassen.lo mzp.lo triangular.lo triangular_russian.lo ple.lo ple_russian.lo solve.lo echelonform.lo mmc.lo debug_dump.lo io
.lo -lm -lpng
libtool: link: gcc -shared  -fPIC -DPIC  .libs/brilliantrussian.o .libs/misc.o .libs/mzd.o .libs/graycode.o .libs/strassen.o .libs/mzp.o .
libs/triangular.o .libs/triangular_russian.o .libs/ple.o .libs/ple_russian.o .libs/solve.o .libs/echelonform.o .libs/mmc.o .libs/debug_dum
p.o .libs/io.o   -lm -lpng  -mmmx -msse -msse2 -msse3 -O2   -Wl,-soname -Wl,libm4ri-0.0.20111203.so -o .libs/libm4ri-0.0.20111203.so
/usr/bin/ld: cannot find -lpng
collect2: ld returned 1 exit status
make[1]: *** [libm4ri.la] Error 1
make[1]: Leaving directory `/home/buildbot/build/sage/eno-1/eno_full/build/sage-5.1.beta1/spkg/build/libm4ri-20120415/src'
make: *** [all] Error 2
Error building libm4ri
$ ls -l local/lib/libpng*
36k -rw-r--r-- 1 buildbot sage 828k May  8 06:16 local/lib/libpng12.a
4.1k -rwxr-xr-x 1 buildbot sage 1.1k May  8 06:16 local/lib/libpng12.la*
   0 lrwxrwxrwx 1 buildbot sage   18 May  8 06:16 local/lib/libpng12.so -> libpng12.so.0.35.0*
   0 lrwxrwxrwx 1 buildbot sage   18 May  8 06:16 local/lib/libpng12.so.0 -> libpng12.so.0.35.0*
467k -rwxr-xr-x 1 buildbot sage 460k May  8 06:16 local/lib/libpng12.so.0.35.0*

$ ls -l /lib*/libpng* /usr/lib*/libpng*
ls: cannot access /lib*/libpng*: No such file or directory
   0 lrwxrwxrwx 1 root root   16 May  2 11:53 /usr/lib64/libpng.so.3 -> libpng.so.3.49.0*
173k -rwxr-xr-x 1 root root 172k Apr  7 14:33 /usr/lib64/libpng.so.3.49.0*
   0 lrwxrwxrwx 1 root root   18 May  2 11:53 /usr/lib64/libpng12.so.0 -> libpng12.so.0.49.0*
164k -rwxr-xr-x 1 root root 162k Apr  7 14:33 /usr/lib64/libpng12.so.0.49.0*

Note that there is no file /usr/lib64/libpng.so, so obviously there cannot be proper PNG support on this machine. I consider this to be an upstream (not Sage) error.

$ grep -i png spkg/logs/libm4ri-20120415.log
checking whether to build with PNG support... yes
checking for PNG... no
checking for PNG... yes
/bin/sh ./libtool --tag=CC   --mode=link gcc -std=gnu99 -mmmx -msse -msse2 -msse3   -g -fPIC -Wall -pedantic -O2 -release 0.0.20111203 -no-undefined  -o libm4ri.la -rpath /home/buildbot/build/sage/eno-1/eno_full/build/sage-5.1.beta1/local/lib brilliantrussian.lo misc.lo mzd.lo graycode.lo strassen.lo mzp.lo triangular.lo triangular_russian.lo ple.lo ple_russian.lo solve.lo echelonform.lo mmc.lo debug_dump.lo io.lo -lm -lpng
libtool: link: gcc -shared  -fPIC -DPIC  .libs/brilliantrussian.o .libs/misc.o .libs/mzd.o .libs/graycode.o .libs/strassen.o .libs/mzp.o .libs/triangular.o .libs/triangular_russian.o .libs/ple.o .libs/ple_russian.o .libs/solve.o .libs/echelonform.o .libs/mmc.o .libs/debug_dump.o .libs/io.o   -lm -lpng  -mmmx -msse -msse2 -msse3 -O2   -Wl,-soname -Wl,libm4ri-0.0.20111203.so -o .libs/libm4ri-0.0.20111203.so
/usr/bin/ld: cannot find -lpng

@jhpalmieri
Copy link
Member

Changed dependencies from to be merged with #12841 to to be merged with #12841, #13109

@jhpalmieri

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Jun 30, 2012

comment:55

The deprecations are not doctested.

@vbraun
Copy link
Member

vbraun commented Jun 30, 2012

comment:56

Hmm deprecations (and any other warnings) don't work in cdef methods. I guess we can't doctest them, for now.

@vbraun
Copy link
Member

vbraun commented Jun 30, 2012

Changed author from Martin Albrecht to Martin Albrecht, John Palmieri

@jdemeyer jdemeyer removed this from the sage-5.2 milestone Jul 1, 2012
@jdemeyer jdemeyer added this to the sage-5.3 milestone Jul 27, 2012
@jdemeyer jdemeyer removed the pending label Jul 27, 2012
@jdemeyer
Copy link

comment:59

This needs to be rebased to sage-5.2.rc1.

@jhpalmieri
Copy link
Member

comment:60

Can you clarify the rebasing issues? I have no problems applying the patches from here and from #12841 on top of sage-5.2.rc1.

@vbraun
Copy link
Member

vbraun commented Jul 28, 2012

comment:61

I'm with John here, I've been carrying the mari/e/givaro/linbox patches in my queue for a while now and there was no breakage in sage-5.2.rc1 as far as I can tell.

@jdemeyer
Copy link

comment:62

My mistake, I only applied the second Sage library patch and that didn't work.

@jdemeyer
Copy link

jdemeyer commented Aug 1, 2012

Merged: sage-5.3.beta0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants