#24214 closed enhancement (fixed)
Upgrade to givaro-4.0.4 fflas-ffpack-2.3.2 and LinBox-1.5.2
Reported by: | cpernet | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | packages: standard | Keywords: | linear algebra, linbox, givaro, fflas-ffpack |
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/linbox-team/givaro/releases/download/v4.0.4/givaro-4.0.4.tar.gz
- https://github.com/linbox-team/fflas-ffpack/releases/download/v2.3.2/fflas_ffpack-2.3.2.tar.bz2
- https://github.com/linbox-team/linbox/releases/download/v1.5.2/linbox-1.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 spkg-install'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 follow-up: ↓ 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 givaro-4.0.3 fflas-ffpack-2.3.0 and LinBox-1.5.0 to Upgrade to givaro-4.0.4 fflas-ffpack-2.3.1 and LinBox-1.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 sage-on-distro, 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 sage-on-distro maintainer can follow progress on that work?
comment:23 follow-up: ↓ 63 Changed 5 years ago by
For the moment there is this issue https://github.com/linbox-team/linbox/issues/69 but still no working branch. It will very likely be addressed in the next LinBox? meeting mid-december.
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 sage-8.1 to sage-8.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/linbox-team/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 follow-up: ↓ 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.
[sagelib-8.1.rc4] In file included from build/cythonized/sage/libs/linbox/linbox_flint_interface.cpp:647:0: [sagelib-8.1.rc4] /home/vdelecro/sage_patchbot/local/include/linbox/linbox-config.h:31:27: fatal error: linbox/config.h: No such file or directory [sagelib-8.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 ; follow-up: ↓ 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 givaro-4.0.4 fflas-ffpack-2.3.1 and LinBox-1.5.1 to Upgrade to givaro-4.0.4 fflas-ffpack-2.3.1 and LinBox-1.5.2
I released linbox-1.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 64-bit:
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 -fabi-version=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 linbox-sage.lo -MD -MP -MF .deps/linbox-sage.Tpo -c linbox-sage.C -fPIC -DPIC -o .libs/linbox-sage.o In file included from /home/buildbot/slave/sage_git/build/local/include/fflas-ffpack/fflas/fflas_simd.h:208:0, from /home/buildbot/slave/sage_git/build/local/include/fflas-ffpack/fflas/fflas_freduce.h:33, from /home/buildbot/slave/sage_git/build/local/include/fflas-ffpack/fflas/fflas.h:104, from /home/buildbot/slave/sage_git/build/local/include/fflas-ffpack/ffpack/ffpack.h:47, from ../../linbox/matrix/matrixdomain/blas-matrix-domain.h:45, from ../../linbox/matrix/matrix-domain.h:68, from ../../linbox/matrix/sparsematrix/sparse-generic.h:80, from ../../linbox/matrix/sparse-matrix.h:70, from linbox-sage.C:35: /home/buildbot/slave/sage_git/build/local/include/fflas-ffpack/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/fflas-ffpack/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/fflas-ffpack/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/fflas-ffpack/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/fflas-ffpack/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 'linbox-sage.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 fflas-ffpack 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 spkg-install 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/spkg-install
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 pkg-config cflags list (fixing the gcc-4.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_ffpack-2.3.1] config.status: executing depfiles commands [fflas_ffpack-2.3.1] config.status: executing libtool commands [fflas_ffpack-2.3.1] Building fflas_ffpack-2.3.1 [fflas_ffpack-2.3.1] make[3]: Entering directory `/home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack-2.3.1/src' [fflas_ffpack-2.3.1] CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack-2.3.1/src/build-aux/missing aclocal-1.15 -I macros [fflas_ffpack-2.3.1] /home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack-2.3.1/src/build-aux/missing: line 81: aclocal-1.15: command not found [fflas_ffpack-2.3.1] WARNING: 'aclocal-1.15' is missing on your system. [fflas_ffpack-2.3.1] You should only need it if you modified 'acinclude.m4' or [fflas_ffpack-2.3.1] 'configure.ac' or m4 files included by 'configure.ac'. [fflas_ffpack-2.3.1] The 'aclocal' program is part of the GNU Automake package: [fflas_ffpack-2.3.1] <http://www.gnu.org/software/automake> [fflas_ffpack-2.3.1] It also requires GNU Autoconf, GNU m4 and Perl in order to run: [fflas_ffpack-2.3.1] <http://www.gnu.org/software/autoconf> [fflas_ffpack-2.3.1] <http://www.gnu.org/software/m4/> [fflas_ffpack-2.3.1] <http://www.perl.org/> [fflas_ffpack-2.3.1] make[3]: *** [aclocal.m4] Error 127 [fflas_ffpack-2.3.1] make[3]: Failed to remake makefile `Makefile'. [fflas_ffpack-2.3.1] make[3]: Target `all' not remade because of errors. [fflas_ffpack-2.3.1] make[3]: Leaving directory `/home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/fflas_ffpack-2.3.1/src' [fflas_ffpack-2.3.1] ******************************************************************************** [fflas_ffpack-2.3.1] Error building fflas_ffpack-2.3.1 [fflas_ffpack-2.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 fflas-ffpack 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 givaro-4.0.4 fflas-ffpack-2.3.1 and LinBox-1.5.2 to Upgrade to givaro-4.0.4 fflas-ffpack-2.3.2 and LinBox-1.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 fflas-ffpack (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 follow-up: ↓ 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 gh-timokau:
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 ; follow-up: ↓ 64 Changed 4 years ago by
Replying to fbissey:
Replying to gh-timokau:
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/linbox-team/linbox/issues/69 but still no working branch. It will very likely be addressed in the next LinBox? meeting mid-december.
Was there any progress in that meeting or in general?
comment:64 in reply to: ↑ 63 Changed 4 years ago by
Replying to gh-timokau:
Replying to cpernet:
For the moment there is this issue https://github.com/linbox-team/linbox/issues/69 but still no working branch. It will very likely be addressed in the next LinBox? meeting mid-december.
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 fflas-ffpack and LinBox
latest release candidates