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 LinBox to most recent upstream release #12883

Closed
pcpa mannequin opened this issue Apr 26, 2012 · 106 comments
Closed

Update LinBox to most recent upstream release #12883

pcpa mannequin opened this issue Apr 26, 2012 · 106 comments

Comments

@pcpa
Copy link
Mannequin

pcpa mannequin commented Apr 26, 2012

The version of LinBox used in Sage is 4 years old, we should update.

  1. Observe: Dependencies
  2. **Install: **http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.6.0.spkg
  3. Install: http://boxen.math.washington.edu/home/jdemeyer/spkg/linbox-1.3.2.spkg
  4. Apply: attachment: matrix_modn_dense_no_linbox.patch
  5. Apply: attachment: sage-linbox.2.patch
  6. Apply: attachment: trac_12883_linbox_deps.patch and attachment: trac_12883-spkg-install.patch to SAGE_ROOT repository
  7. Apply: attachment: trac_12883_hg_ignore_fflas_config.patch to the scripts (SAGE_LOCAL/bin) repository.

SPKG Repositories:

  1. https://bitbucket.org/malb/fflas-ffpack-spkg
  2. https://bitbucket.org/malb/linbox-spkg

Note for release manager: This ticket must be merged simultaneously with #13164, they depend on each other.

Depends on #13118
Depends on #12840
Depends on #12841
Depends on #13164

Upstream: Fixed upstream, but not in a stable release.

CC: @jpflori

Component: linbox

Author: Paulo César Pereira de Andrade, Martin Albrecht

Reviewer: Volker Braun, Jeroen Demeyer

Merged: sage-5.4.beta1

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

@pcpa pcpa mannequin added this to the sage-5.0 milestone Apr 26, 2012
@pcpa pcpa mannequin assigned ClementPernet Apr 26, 2012
@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 26, 2012

comment:1

What's the title supposed to tell us (if you get build errors with that version)?

For GCC 4.7.x / C++11 issues see #12762; for (problems with) updating LinBox see #11718. (I think I've read about complications when trying to upgrade to 1.2.2 elsewhere as well.) See #12865 for yet another issue, which is still present in 1.2.2.

@pcpa
Copy link
Mannequin Author

pcpa mannequin commented Apr 26, 2012

comment:2

Sorry for lack of a better description. I am trying to address and submit reports to involved upstream parties about some patches and version issues of some components, while checking how far I can go building sagemath in Fedora, and using Fedora packages.

For example, the first error:

error: 'linbox_modn_dense_echelonize' was not declared in this scope

in /usr/include/linbox/linbox-sage.h I see only

EXTERN unsigned long int linbox_modn_dense_echelonize_double (double modulus, double*matrix, size_t nrows, size_t ncols);

EXTERN unsigned long int linbox_modn_dense_echelonize_float (float modulus, float * matrix,size_t nrows, size_t ncols);

Same happens for linbox_modn_dense_delete_array that now have prototypes for linbox_modn_dense_delete_array_double and linbox_modn_dense_delete_array_float.

Also linbox_integer_dense_matrix_matrix_multiply changed prototype from

/* ans must be a pre-allocated and pre-initialized array of GMP ints. */
EXTERN int linbox_integer_dense_matrix_matrix_multiply(mpz_t** ans, mpz_t **A, mpz_t **B,
			      size_t A_nr, size_t A_nc, size_t B_nr, size_t B_nc);

in the sagemath linbox-1.1.6 spkg, to

/* ans must be a pre-allocated and pre-initialized array of GMP ints. */
int linbox_integer_dense_matrix_matrix_multiply (mpz_t** ans, mpz_t **A, mpz_t **B, size_t m, size_t n, size_t k);

in linbox-1.2.2.

I am trying to have upstream aware of these issues, and if not possible to get components working, I can try to get an extra linbox 1.1.6 package in Fedora.

@pcpa
Copy link
Mannequin Author

pcpa mannequin commented Apr 26, 2012

comment:3

Upstream linbox 1.2.2 should need this patch

--- linbox-1.2.2/interfaces/sage/linbox-sage.h.orig     2012-04-26 16:25:37.500132509 -0300
+++ linbox-1.2.2/interfaces/sage/linbox-sage.h  2012-04-26 16:26:00.705133519 -0300
@@ -48,7 +48,9 @@
 EXTERN void linbox_modn_dense_delete_array_double (double *f);
 EXTERN void linbox_modn_dense_delete_array_float (float *f);
 
-
+template <class Element>
+unsigned long int linbox_modn_dense_echelonize (Element modulus, Element* matrix,
+                                               size_t nrows, size_t ncols);
 EXTERN unsigned long int linbox_modn_dense_echelonize_double (double modulus, double*matrix, size_t nrows, size_t ncols);
 EXTERN unsigned long int linbox_modn_dense_echelonize_float (float modulus, float * matrix,size_t nrows, size_t ncols);
 

But it is not yet enough because the sage matrix interface was changed from "jagged pointers" to a single sequential vector.

I believe I could make a patch for sage/libs/linbox/linbox.{pxd,pyx} but I do not feel confident about other places where sage interfaces to linbox matrices.

@pcpa
Copy link
Mannequin Author

pcpa mannequin commented Apr 29, 2012

Experimental linbox 1.2.2 patch

@pcpa
Copy link
Mannequin Author

pcpa mannequin commented Apr 29, 2012

comment:4

Attachment: sage-4.8-linbox.patch.gz

The attached patches allow continuing experimenting with a new sagemath rpm.
The patches are only tested to know they make sagemath and linbox agree on api/abi, but no testing done.
There were missing prototypes/templates in linbox-sage.h as well as wrong ones.
I am not fully confident that some place may do wrong dereferencing of matrix fields due to upstream change from a vector of rows where every row is an allocated pointer to a single memory chunk.
Another part I am not confident is that I did the correct logic in the change from
A_nr, A_nc, B_nr, B_nc to test in sage code if A_nr != B_nc: ..., and convert the values of m, n, k as
A_nr, A_nc, B_nr.

This is not a proposal of a final patch, but a extra call for more feedback. As this was also the first time I actually did look at linbox code, so, just patched the interfaces to make both sides agree.

@pcpa
Copy link
Mannequin Author

pcpa mannequin commented May 5, 2012

fflas-ffpack-64bit.patch

@pcpa
Copy link
Mannequin Author

pcpa mannequin commented May 5, 2012

Attachment: fflas-ffpack-64bit.patch.gz

Attachment: linbox-sagemath.patch.gz

linbox-sagemath.patch

@pcpa
Copy link
Mannequin Author

pcpa mannequin commented May 5, 2012

comment:5

Is it reasonably to define mod_int to C long?

If not, safest approach should be to use uint32_t I believe, otherwise, should require coding 64 bit templates in linbox 1.2.2.

The new attached fflas-ffpack-64bit.patch is required to generate proper 64 bit code, and the updated linbox-sagemath.patch should be very close to the final format, but note that in 64 bit arch, linbox 1.2.2 with the attached patches needs also to have -D!__LINBOX_HAVE_INT64=1 in CFLAGS and CXXFLAGS.

I should update the sage-4.8-linbox.patch at some point, as the initial patch I attached was only enough to get to another build failure in Fedora. But the next update should be for sage-5.0.

@pcpa pcpa mannequin removed this from the sage-5.0 milestone May 5, 2012
@pcpa pcpa mannequin added the wishlist item label May 5, 2012
@malb
Copy link
Member

malb commented Jun 6, 2012

comment:6

I cut new SPKGs for LinBox and FFLAS/FFPACK:

http://sage.math.washington.edu/home/malb/spkgs/fflas_ffpack-1.5.0.spkg

http://sage.math.washington.edu/home/malb/spkgs/linbox-1.3.0.spkg

I am tracking SPKG repositories here:

https://bitbucket.org/malb/fflas-ffpack-spkg

https://bitbucket.org/malb/linbox-spkg

I will go through your patches now.

@malb malb changed the title Please consider updating to linbox 1.2.2 Update LinBox to most recent upstream release Jun 6, 2012
@malb

This comment has been minimized.

@malb

This comment has been minimized.

@malb
Copy link
Member

malb commented Jun 7, 2012

Dependencies: #12840, 12841, #9511

@malb

This comment has been minimized.

@malb
Copy link
Member

malb commented Jun 7, 2012

Author: Paulo César Pereira de Andrade, Martin Albrecht

@malb

This comment has been minimized.

@malb

This comment has been minimized.

@malb
Copy link
Member

malb commented Jun 7, 2012

Changed dependencies from #12840, 12841, #9511 to #12840, #12841, #9511

@malb

This comment has been minimized.

@malb

This comment has been minimized.

@malb

This comment has been minimized.

@malb

This comment has been minimized.

@malb
Copy link
Member

malb commented Jun 16, 2012

comment:17

All doctests pass on geom.math and my machine with 5.0.1.

@jhpalmieri
Copy link
Member

comment:78

Should the references to #9511 be changed to #13164 (in the dependencies and the ticket description?

@vbraun
Copy link
Member

vbraun commented Aug 25, 2012

Changed dependencies from #13118, #12840, #12841, #9511 to #13118, #12840, #12841, #13164

@jdemeyer
Copy link

comment:80

Replying to @vbraun:

I also like how the installation guide is dancing around the fact that CFLAG64 is currently useless since it is not used in all spgks, so if you were to use a compiler that doesn't understand its default -m64 value breakage will ensue. So I'll concede that it is documented, but it still doesn't sound like a good idea to me.

This is all very true, but no good excuse for sloppy rebasing...

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Sep 5, 2012

comment:82

Rebased from scratch, needs review.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Sep 5, 2012

Diff for linbox 1.1.6.p11->1.3.2. For reference / review only.

@jdemeyer
Copy link

comment:84

Attachment: linbox-1.3.2.diff.gz

@jdemeyer
Copy link

Merged: sage-5.4.beta1

@jdemeyer
Copy link

jdemeyer commented Oct 8, 2012

comment:85

Could this cause the following sporadic doctest error:

sage -t  --long -force_lib devel/sage/sage/modular/modform/ambient.py
you are running out of primes. 1000 coprime primes
found**********************************************************************
File
"/release/buildbot/sage/sage-1/sage_upgrade_5.2/build/sage-5.5.beta0/devel/sage-main/sage/modular/modform/ambient.py",
line 815:
    sage: [f[0]%p for p in prime_range(100)] # long time (0s, depends on
above)
Expected:
    [0, 0, 0, 0, 1, 9, 2, 7, 0, 0, 0, 0, 1, 12, 9, 16, 37, 0, 21, 11,
70, 22, 0, 58, 76]
Got:
    [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1, 1, 1]
**********************************************************************
File
"/release/buildbot/sage/sage-1/sage_upgrade_5.2/build/sage-5.5.beta0/devel/sage-main/sage/modular/modform/ambient.py",
line 817:
    sage: [f[42]%p for p in prime_range(100)] # long time (0s, depends
on above)
Expected:
    [0, 0, 4, 0, 10, 4, 4, 8, 12, 1, 23, 13, 10, 27, 20, 13, 16, 59, 53,
41, 11, 13, 12, 6, 82]
Got:
    [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0]
**********************************************************************

(the string "you are running out of primes" certainly comes from the new LinBox package).

@vbraun
Copy link
Member

vbraun commented Oct 8, 2012

comment:86

I've posted this question to the linbox-use mailinglist https://groups.google.com/d/topic/linbox-use/SgsXVYM7u7s/discussion

@jdemeyer
Copy link

jdemeyer commented Oct 8, 2012

comment:87

FYI: I did 4000 doctest runs and this happened 9 times, so it is quite rare.

Because of the rarity, it doesn't have to be a sage-5.4 blocker, but of course I'd like to see it fixed.

@jdemeyer
Copy link

jdemeyer commented Oct 8, 2012

comment:88

With previous LinBox versions, this file sometimes (equally rarely) timeouts in doctests. So maybe it's the same issue, but with a timeout before, and an error now.

@jdemeyer
Copy link

jdemeyer commented Nov 2, 2012

comment:89

Unfortunately, no response from upstream concerning the "running out of primes" error.

@jdemeyer
Copy link

comment:90

Either this or #13164 caused #14032.

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

7 participants