#24214 closed enhancement (fixed)
Upgrade to givaro4.0.4 fflasffpack2.3.2 and LinBox1.5.2
Reported by:  cpernet  Owned by:  

Priority:  major  Milestone:  sage8.2 
Component:  packages: standard  Keywords:  linear algebra, linbox, givaro, fflasffpack 
Cc:  dlucas, fbissey  Merged in:  
Authors:  Clément Pernet  Reviewers:  Volker Braun 
Report Upstream:  N/A  Work issues:  
Branch:  3aad376 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Releases can be found at
 https://github.com/linboxteam/givaro/releases/download/v4.0.4/givaro4.0.4.tar.gz
 https://github.com/linboxteam/fflasffpack/releases/download/v2.3.2/fflas_ffpack2.3.2.tar.bz2
 https://github.com/linboxteam/linbox/releases/download/v1.5.2/linbox1.5.2.tar.gz
This ticket fixes:
Attachments (4)
Change History (68)
comment:1 Changed 5 years ago by
 Cc fbissey added
comment:2 Changed 5 years ago by
 Branch set to u/cpernet/upgrade_to_givaro_4_0_3_fflas_ffpack_2_3_0_and_linbox_1_5_0
comment:3 Changed 5 years ago by
 Commit set to 7f14656c470e82888f29238f69a43933a75465d9
 Description modified (diff)
comment:4 Changed 5 years ago by
 Description modified (diff)
comment:5 Changed 5 years ago by
 Description modified (diff)
comment:6 Changed 5 years ago by
 Description modified (diff)
comment:7 Changed 5 years ago by
 Commit changed from 7f14656c470e82888f29238f69a43933a75465d9 to 63febb299cc03ff06c02511789a75efb55057290
Branch pushed to git repo; I updated commit sha1. New commits:
63febb2  update to latest release and remove obsolete bug fix

comment:8 Changed 5 years ago by
 Commit changed from 63febb299cc03ff06c02511789a75efb55057290 to 809ed54eee76a9685ba201928c36deaedbd76952
Branch pushed to git repo; I updated commit sha1. New commits:
809ed54  new checksums

comment:9 Changed 5 years ago by
 Description modified (diff)
comment:10 Changed 5 years ago by
All three libraries have been released. The code in this ticket's branch works fine for me with these new releases. I still need to do one cleanup pass on the spkginstall's and document SPKG.txt before opening it for review.
comment:11 Changed 5 years ago by
 Description modified (diff)
comment:12 Changed 5 years ago by
 Commit changed from 809ed54eee76a9685ba201928c36deaedbd76952 to 99f46017a5cd93084c3cb30a41c29b4a8b99c1fd
comment:13 Changed 5 years ago by
 Commit changed from 99f46017a5cd93084c3cb30a41c29b4a8b99c1fd to ae28c1e1ccf7e29fcf97881ff5c4749d6e5c90f1
Branch pushed to git repo; I updated commit sha1. New commits:
ae28c1e  add checksums for latest releases

comment:14 Changed 5 years ago by
 Commit changed from ae28c1e1ccf7e29fcf97881ff5c4749d6e5c90f1 to 438f6ba5fd901f47e269fbdae90f4cdf7668ee13
Branch pushed to git repo; I updated commit sha1. New commits:
438f6ba  update md5sums

comment:15 Changed 5 years ago by
 Commit changed from 438f6ba5fd901f47e269fbdae90f4cdf7668ee13 to 5c930384e43a340fc36d20e2b9eecd02a91047f2
Branch pushed to git repo; I updated commit sha1. New commits:
5c93038  add charpoly_CRA fixing #15535 and #21579

comment:16 Changed 5 years ago by
 Description modified (diff)
comment:17 followup: ↓ 22 Changed 5 years ago by
Note for the reviewer: after some discussions, we (LinBox? dev) decided not to merge the patch fixing #21579 and #15535 (replacing the probabilistic early terminated Chinese Remainder Algorithm for charpoly by a deterministic one) in the release and we therefore add the patch in sage. This is because there was no consensus that LinBox? should default to the deterministic version of the algorithm. We will rework the interface to allow a switch between the two variants, and the option to pass a bound on the probability of failure for the probabilistic algorithm, but meanwhile, the decision was to not change the default (probabilistic) in release 1.5.1.
comment:18 Changed 5 years ago by
 Commit changed from 5c930384e43a340fc36d20e2b9eecd02a91047f2 to dfc0b846e493ebb17af64122a117c13244746589
Branch pushed to git repo; I updated commit sha1. New commits:
dfc0b84  remove obsolete check

comment:19 Changed 5 years ago by
 Status changed from new to needs_review
comment:20 Changed 5 years ago by
 Description modified (diff)
comment:21 Changed 5 years ago by
 Summary changed from Upgrade to givaro4.0.3 fflasffpack2.3.0 and LinBox1.5.0 to Upgrade to givaro4.0.4 fflasffpack2.3.1 and LinBox1.5.1
comment:22 in reply to: ↑ 17 Changed 5 years ago by
Replying to cpernet:
Note for the reviewer: after some discussions, we (LinBox? dev) decided not to merge the patch fixing #21579 and #15535 (replacing the probabilistic early terminated Chinese Remainder Algorithm for charpoly by a deterministic one) in the release and we therefore add the patch in sage. This is because there was no consensus that LinBox? should default to the deterministic version of the algorithm. We will rework the interface to allow a switch between the two variants, and the option to pass a bound on the probability of failure for the probabilistic algorithm, but meanwhile, the decision was to not change the default (probabilistic) in release 1.5.1.
Right. It is not completely unreasonable. However it has consequence for sageondistro, where the linbox maintainer may prefer to leave linbox "vanilla" instead of patching it the sage way. Is there an issue opened for the switching version or a branch open where I and my fellow sageondistro maintainer can follow progress on that work?
comment:23 followup: ↓ 63 Changed 5 years ago by
For the moment there is this issue https://github.com/linboxteam/linbox/issues/69 but still no working branch. It will very likely be addressed in the next LinBox? meeting middecember.
Changed 5 years ago by
Changed 5 years ago by
Changed 5 years ago by
comment:24 Changed 5 years ago by
 Milestone changed from sage8.1 to sage8.2
 Status changed from needs_review to needs_work
You introduced a TAB in linbox_flint_interface.pyx
.
More dramatic, on the following doctests my computer get stuck and finally segfault
src/sage/matrix/matrix_integer_dense.pyx
line 1212 (matrix_integer_dense.log)matrix([]).charpoly('y', 'linbox')
src/sage/matrix/matrix_rational_dense.pyx
line 1034 (matrix_rational_dense.log)for _ in range(100): dim = randint(0, 10) m = random_matrix(QQ, dim, num_bound=8, den_bound=8) p_flint = m.charpoly(algorithm='flint'); m._clear_cache() p_linbox = m.charpoly(algorithm='linbox'); m._clear_cache() p_generic = m.charpoly(algorithm='generic') assert p_flint == p_linbox == p_generic
src/sage/graphs/graph.py
line 7717 (graph.log)G = graphs.RandomTree(10) G.ihara_zeta_function_inverse()
For each problematic file, the log is the output of sage t
(stdout+stderr).
comment:25 Changed 5 years ago by
As each time it looks like an error on 0x0 characteristic polynomials, I believe that the cause is around
 a/src/sage/libs/linbox/linbox_flint_interface.pyx +++ b/src/sage/libs/linbox/linbox_flint_interface.pyx  # FIXME: bug in LinBox  # see https://github.com/linboxteam/linbox/issues/51  if fmpz_mat_nrows(A) == 0:  fmpz_poly_one(mp)  return
comment:26 Changed 5 years ago by
 Commit changed from dfc0b846e493ebb17af64122a117c13244746589 to 4c1474a6c04ddfab86c79b2ab6809ebfdfba3d49
Branch pushed to git repo; I updated commit sha1. New commits:
4c1474a  remove tab character

comment:27 Changed 5 years ago by
 Commit changed from 4c1474a6c04ddfab86c79b2ab6809ebfdfba3d49 to 15712e78b9ef5e751cc1a293610f6a16d08ab2ef
Branch pushed to git repo; I updated commit sha1. New commits:
15712e7  fix charpoly of 0x0 matrix bug

comment:28 Changed 5 years ago by
 Status changed from needs_work to needs_review
Apologies for these 2 mistakes. I have fixed the tab and added a fix to the 0x0 charpoly bug in the linbox patch which is on the branch of this ticket. It fixes all three bugs you listed, on the a box where I could reproduce them.
comment:29 followup: ↓ 38 Changed 5 years ago by
I seem to be getting a consistent timeout on src/sage/modular/modform/ambient.py
I'll investigate further...
Changed 5 years ago by
comment:30 Changed 5 years ago by
Twice trying to build from scratch on 8.1.rc4 with this branch applied I faced the same trouble with boost... weird. Full log is install.log.gz.
comment:31 Changed 5 years ago by
boost
is a red herring.
[sagelib8.1.rc4] In file included from build/cythonized/sage/libs/linbox/linbox_flint_interface.cpp:647:0: [sagelib8.1.rc4] /home/vdelecro/sage_patchbot/local/include/linbox/linboxconfig.h:31:27: fatal error: linbox/config.h: No such file or directory [sagelib8.1.rc4] compilation terminated.
comment:32 Changed 5 years ago by
good catch :)
comment:33 Changed 5 years ago by
It helps to do serial build of the package in that case.
comment:34 Changed 5 years ago by
How is this even possible? Do you know which parallel build is broken? Is it a simple dependency problem (I see $(inst_linbox)
in build/make/deps
)?
comment:35 Changed 5 years ago by
Sorry I did not make myself clear. I meant when you meet the error, restart the build serially. That way you have a clear idea of where the failure is. Which file is the culprit. Here I found the error message but I don't know where the associated compilation line is.
comment:36 Changed 5 years ago by
Ok, sorry again. This is indeed a bug, that config.h no longer gets installed. I pushed a patch fixing it to get this ticket rolling, but it is annoying enough, that I'll try to cut a bug fix release in a day or two with this fix.
comment:37 Changed 5 years ago by
 Commit changed from 15712e78b9ef5e751cc1a293610f6a16d08ab2ef to 3b8df4579db34c01fecfb704f18edb8d2a5b1f43
Branch pushed to git repo; I updated commit sha1. New commits:
3b8df45  add fix for 24214 missing config.h

comment:38 in reply to: ↑ 29 ; followup: ↓ 39 Changed 5 years ago by
 Status changed from needs_review to needs_work
Replying to jdemeyer:
I seem to be getting a consistent timeout on
src/sage/modular/modform/ambient.py
Confirmed. This seems to hang forever:
sage: t = ModularForms(1, 512)._compute_hecke_matrix(5); t.charpoly()
Note: the matrix has size only 43x43 but it has very large coefficients.
comment:39 in reply to: ↑ 38 Changed 5 years ago by
Much simpler hang (a 1x1 matrix with a single huge entry):
sage: Matrix(1, 1, [2^2000]).charpoly()
comment:40 Changed 5 years ago by
 Commit changed from 3b8df4579db34c01fecfb704f18edb8d2a5b1f43 to a843f48b7a4267e44895a3dfa892c89c85b85611
Branch pushed to git repo; I updated commit sha1. New commits:
a843f48  update LinBox patch to fix charpoly of [2^2000] bug

comment:41 Changed 5 years ago by
I pushed a fix for this bug. I fixes both instances of #comment:38 and #comment:39 for me.
comment:42 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:43 Changed 5 years ago by
At least all tests pass on quasar (make ptestlong
)!
comment:44 Changed 5 years ago by
 Commit changed from a843f48b7a4267e44895a3dfa892c89c85b85611 to 8e0246e4b7f3478d5952c776de358becfa6f573c
Branch pushed to git repo; I updated commit sha1. New commits:
8e0246e  update to 1.5.2 release (merging the missing config.h patch)

comment:45 Changed 5 years ago by
 Description modified (diff)
 Summary changed from Upgrade to givaro4.0.4 fflasffpack2.3.1 and LinBox1.5.1 to Upgrade to givaro4.0.4 fflasffpack2.3.1 and LinBox1.5.2
I released linbox1.5.2 which includes the fix to the missing config.h file, and updated the branch accordingly. Everything is now fine on my side.
comment:46 Changed 5 years ago by
 Status changed from needs_review to needs_work
Merge conflicts on 8.2.beta0
comment:47 Changed 5 years ago by
 Branch changed from u/cpernet/upgrade_to_givaro_4_0_3_fflas_ffpack_2_3_0_and_linbox_1_5_0 to u/vbraun/upgrade_to_givaro_4_0_3_fflas_ffpack_2_3_0_and_linbox_1_5_0
comment:48 Changed 5 years ago by
 Commit changed from 8e0246e4b7f3478d5952c776de358becfa6f573c to 4194fd4f7b6bfcd3d89b1998dd4540e5d676782d
 Status changed from needs_work to needs_review
New commits:
4194fd4  Merge tag '8.2.beta0' into t/24214/upgrade_to_givaro_4_0_3_fflas_ffpack_2_3_0_and_linbox_1_5_0

comment:49 Changed 5 years ago by
all tests pass for me (excepted #24396 that also fails on develop)
comment:50 Changed 5 years ago by
 Status changed from needs_review to needs_work
On Debian 8 64bit:
libtool: compile: g++ DHAVE_CONFIG_H I. I../.. DDISABLE_COMMENTATOR I../.. DFFLAS_COMPILED DFFPACK_COMPILED I/home/buildbot/slave/sage_git/build/local/include std=gnu++11 fabiversion=6 I/home/buildbot/slave/sage_git/build/local/include I/home/buildbot/slave/sage_git/build/local/include I/home/buildbot/slave/sage_git/build/local/include I/home/buildbot/slave/sage_git/build/local/include O2 Wall g DNDEBUG U_LB_DEBUG DDISABLE_COMMENTATOR std=gnu++11 msse msse2 msse3 mssse3 msse4.1 msse4.2 mavx mavx2 mfma MT linboxsage.lo MD MP MF .deps/linboxsage.Tpo c linboxsage.C fPIC DPIC o .libs/linboxsage.o In file included from /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas_simd.h:208:0, from /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas_freduce.h:33, from /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas.h:104, from /home/buildbot/slave/sage_git/build/local/include/fflasffpack/ffpack/ffpack.h:47, from ../../linbox/matrix/matrixdomain/blasmatrixdomain.h:45, from ../../linbox/matrix/matrixdomain.h:68, from ../../linbox/matrix/sparsematrix/sparsegeneric.h:80, from ../../linbox/matrix/sparsematrix.h:70, from linboxsage.C:35: /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas_simd/simd256.inl: In static member function 'static Simd256i_base::vect_t Simd256i_base::sll128(Simd256i_base::vect_t)': /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas_simd/simd256.inl:102:85: error: there are no arguments to '_mm256_bslli_epi128' that depend on a template parameter, so a declaration of '_mm256_bslli_epi128' must be available [fpermissive] static INLINE CONST vect_t sll128(const vect_t a) { return _mm256_bslli_epi128(a, s); } ^ /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas_simd/simd256.inl:102:85: note: (if you use 'fpermissive', G++ will accept your code, but allowing the use of an undeclared name is deprecated) /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas_simd/simd256.inl: In static member function 'static Simd256i_base::vect_t Simd256i_base::srl128(Simd256i_base::vect_t)': /home/buildbot/slave/sage_git/build/local/include/fflasffpack/fflas/fflas_simd/simd256.inl:110:85: error: there are no arguments to '_mm256_bsrli_epi128' that depend on a template parameter, so a declaration of '_mm256_bsrli_epi128' must be available [fpermissive] static INLINE CONST vect_t srl128(const vect_t a) { return _mm256_bsrli_epi128(a, s); } ^ Makefile:535: recipe for target 'linboxsage.lo' failed
comment:51 Changed 5 years ago by
Isn't it the same problem as in ticket:17635#comment:178 : a bug in debian gcc ? It was fixed in the previous fflasffpack spkg. Maybe the fix was removed in the refactorization of the spkg commands ? The fix could be to either pass fpermissive to gcc or require to use sage's gcc (version >4.9.2)
comment:52 Changed 5 years ago by
Well you removed the fpermissive from spkginstall in 99f46017a5cd93084c3cb30a41c29b4a8b99c1fd...
comment:53 Changed 5 years ago by
 Branch changed from u/vbraun/upgrade_to_givaro_4_0_3_fflas_ffpack_2_3_0_and_linbox_1_5_0 to u/cpernet/upgrade_to_givaro_4_0_3_fflas_ffpack_2_3_0_and_linbox_1_5_0
comment:54 Changed 5 years ago by
 Commit changed from 4194fd4f7b6bfcd3d89b1998dd4540e5d676782d to a841d66c585ac1421d28a695f6c64e5f9d7eb6a7
 Status changed from needs_work to needs_review
Ok, sorry, I got confused.
Indeed I removed this section from fflas_ffpack/spkginstall
since the fix upstream now fixes it.
However, the fpermissive
flag was not exported to the pkgconfig cflags
list, hence LinBox did not see use it.
I pushed a patch in this branch and this is fixed upstream too.
New commits:
a841d66  add patch so that fpermissive option is exported in pkgconfig cflags list (fixing the gcc4.9.2 bug)

comment:55 Changed 5 years ago by
 Status changed from needs_review to needs_work
That then triggers an autoconf which we don't depend on:
[fflas_ffpack2.3.1] config.status: executing depfiles commands [fflas_ffpack2.3.1] config.status: executing libtool commands [fflas_ffpack2.3.1] Building fflas_ffpack2.3.1 [fflas_ffpack2.3.1] make[3]: Entering directory `/home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack2.3.1/src' [fflas_ffpack2.3.1] CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack2.3.1/src/buildaux/missing aclocal1.15 I macros [fflas_ffpack2.3.1] /home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack2.3.1/src/buildaux/missing: line 81: aclocal1.15: command not found [fflas_ffpack2.3.1] WARNING: 'aclocal1.15' is missing on your system. [fflas_ffpack2.3.1] You should only need it if you modified 'acinclude.m4' or [fflas_ffpack2.3.1] 'configure.ac' or m4 files included by 'configure.ac'. [fflas_ffpack2.3.1] The 'aclocal' program is part of the GNU Automake package: [fflas_ffpack2.3.1] <http://www.gnu.org/software/automake> [fflas_ffpack2.3.1] It also requires GNU Autoconf, GNU m4 and Perl in order to run: [fflas_ffpack2.3.1] <http://www.gnu.org/software/autoconf> [fflas_ffpack2.3.1] <http://www.gnu.org/software/m4/> [fflas_ffpack2.3.1] <http://www.perl.org/> [fflas_ffpack2.3.1] make[3]: *** [aclocal.m4] Error 127 [fflas_ffpack2.3.1] make[3]: Failed to remake makefile `Makefile'. [fflas_ffpack2.3.1] make[3]: Target `all' not remade because of errors. [fflas_ffpack2.3.1] make[3]: Leaving directory `/home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack2.3.1/src' [fflas_ffpack2.3.1] ******************************************************************************** [fflas_ffpack2.3.1] Error building fflas_ffpack2.3.1 [fflas_ffpack2.3.1] ********************************************************************************
comment:56 Changed 5 years ago by
 Commit changed from a841d66c585ac1421d28a695f6c64e5f9d7eb6a7 to 3aad376429ee4b3658cbb3d28617fd67988694a1
Branch pushed to git repo; I updated commit sha1. New commits:
3aad376  update to fflasffpack 2.3.2

comment:57 Changed 5 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
 Summary changed from Upgrade to givaro4.0.4 fflasffpack2.3.1 and LinBox1.5.2 to Upgrade to givaro4.0.4 fflasffpack2.3.2 and LinBox1.5.2
Ok, this would be too tedious to patch the automatically generated configure scripts and makefiles. Instead, I just released a minor bug fix release of fflasffpack (v2.3.2), and updated the branch, and the link to the tarball in this ticket accordingly.
comment:58 Changed 5 years ago by
 Reviewers set to Volker Braun
 Status changed from needs_review to positive_review
OK, back for another spin.
comment:59 Changed 5 years ago by
 Branch changed from u/cpernet/upgrade_to_givaro_4_0_3_fflas_ffpack_2_3_0_and_linbox_1_5_0 to 3aad376429ee4b3658cbb3d28617fd67988694a1
 Resolution set to fixed
 Status changed from positive_review to closed
comment:60 Changed 4 years ago by
 Commit 3aad376429ee4b3658cbb3d28617fd67988694a1 deleted
@cpernet if I understand it correctly you are one of the main contributors of linbox. Is there a reason you added linbox_charpoly_fullCRA.patch
as a patch in sages build system instead of applying it upstream? I'm currently building a separate linbox version with that patch just for sage and would like to unify those two.
comment:61 followup: ↓ 62 Changed 4 years ago by
I'm not sure what the commit deleted message means. At least I didn't consciously delete anything.
comment:62 in reply to: ↑ 61 Changed 4 years ago by
Replying to ghtimokau:
I'm not sure what the commit deleted message means. At least I didn't consciously delete anything.
Don't worry about that. It is a trac strange behavior but it has no impact. For your earlier question look at comment 17, 22 and 23 in this ticket.
comment:63 in reply to: ↑ 23 ; followup: ↓ 64 Changed 4 years ago by
Replying to fbissey:
Replying to ghtimokau:
I'm not sure what the commit deleted message means. At least I didn't consciously delete anything.
Don't worry about that. It is a trac strange behavior but it has no impact. For your earlier question look at comment 17, 22 and 23 in this ticket.
Thank you, I should've read the whole thing, I only started reading at comment 40.
Replying to cpernet:
For the moment there is this issue https://github.com/linboxteam/linbox/issues/69 but still no working branch. It will very likely be addressed in the next LinBox? meeting middecember.
Was there any progress in that meeting or in general?
comment:64 in reply to: ↑ 63 Changed 4 years ago by
Replying to ghtimokau:
Replying to cpernet:
For the moment there is this issue https://github.com/linboxteam/linbox/issues/69 but still no working branch. It will very likely be addressed in the next LinBox? meeting middecember.
Was there any progress in that meeting or in general?
No idea, I'll leave it to Clement to answer.
New commits:
updpate version numbers and remove patches merged upstream
upgrade to new polynomial API of fflasffpack and LinBox
latest release candidates