Opened 9 years ago

Last modified 8 years ago

#12835 closed enhancement

upgrade fpLLL to version 4.0.4 — at Version 16

Reported by: malb Owned by: tbd
Priority: major Milestone: sage-5.11
Component: packages: standard Keywords: lll, fplll, spkg
Cc: jpflori Merged in:
Authors: Paulo César Pereira de Andrade, Jean-Pierre Flori Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jpflori)

Sage still ships fpLLL 3.0.12 (we upgraded to 3.0.12 in May 2009!) but fpLLL 4.0.4 is out which includes an implementation of BKZ.

Use spkg at:

Apply:

Change History (20)

comment:1 Changed 9 years ago by Snark

I just checked it won't be painless : sage/libs/fplll/fplll.cpp:243:24: fatal error: fplll/fast.h: No such file or directory

From what I see in upstream's sources:

  • wrapper.h still exists, but isn't installed ;
  • fast.h, fast_earlyred.h, heuristic.h, heuristic_early_red.h and proved.h don't exist anymore, but there's a fplllv31.h which provides a compatibility layer to version 3.1 (which is more recent than what sage has... so perhaps even that isn't enough!)

comment:2 Changed 9 years ago by malb

AFAIK fpLLL 4.0 is essentially a new library from an interface point of view, i.e., they switched to a more C++-style interface. I suggest to essentially start over with the interface instead of trying to patch the old interface.

comment:3 Changed 9 years ago by fbissey

I'd agree with that. A long time ago when fplll-3.1 was released we looked at supporting it in sage-on-gentoo https://github.com/cschwan/sage-on-gentoo/issues/71. Funny thing compiling against 3.0.12 and then upgrading to 3.1.1 without rebuilding sage worked fine.

comment:4 Changed 9 years ago by Snark

There are two points to be made here :

  • I had a better look, and saw wrapper.h's previous features seem to be in fplllv31.h ; but I can't find the "proved" class anywhere. In fact, choosing which method will be used is now done through an enum instead of different classes as far as I see. So indeed a pure rewrite would be the saner route.
  • Apparently, changing the sage-5.1.beta5.spkg and using "sage -f" on it isn't enough : generally sage extracts the spkg in spkg/build/ but really builds sources it copied some previous time in devel/sage/ -- I haven't gotten the logic of it down.

All in all, that makes working on this pretty painful : I'll pass!

Last edited 9 years ago by Snark (previous) (diff)

comment:5 Changed 9 years ago by pcpa

I made sagemath 5.4.beta1 experimental rpm packages for fedora rawhide, patching both fplll and sagemath.

The only issue I noticed so far was:

$ rpm -q sagemath libfplll
sagemath-5.4.beta1-1.fc19.x86_64
libfplll-4.0.1-2.fc19.x86_64
$ sage -t /usr/share/sagemath/devel/sage/sage/libs/fplll/fplll.pyx
sage -t  "devel/sage/sage/libs/fplll/fplll.pyx"             
**********************************************************************
File "/usr/share/sagemath/devel/sage/sage/libs/fplll/fplll.pyx", line 646:
    sage: L = A.LLL(); L
Expected:
    [ 192  264 -152  272   -8  272  -48 -264  104   -8]
    [-128 -176 -240  160 -336  160   32  176  272 -336]
    [ -24 -161  147  350  385  -34  262  161  115  257]
    [ 520   75 -113  -74 -491   54  126  -75  239 -107]
    [-376 -133  255   22  229  150  350  133   95 -411]
    [-168 -103    5  402 -377 -238 -214  103 -219 -249]
    [-352   28  108 -328 -156  184   88  -28  -20  356]
    [ 120 -219  289  298  123  170 -286  219  449 -261]
    [ 160 -292   44   56  164  568  -40  292  -84 -348]
    [ 192  264 -152  272   -8  272  -48  760  104   -8]
Got:
    [ 192  264 -152  272   -8  272  -48 -264  104   -8]
    [-128 -176 -240  160 -336  160   32  176  272 -336]
    [ -24 -161  147  350  385  -34  262  161  115  257]
    [ 520   75 -113  -74 -491   54  126  -75  239 -107]
    [-376 -133  255   22  229  150  350  133   95 -411]
    [-168 -103    5  402 -377 -238 -214  103 -219 -249]
    [-352   28  108 -328 -156  184   88  -28  -20  356]
    [ 120 -219  289  298  123  170 -286  219  449 -261]
    [ 160 -292   44   56  164  568  -40  292  -84 -348]
    [-192  760  152 -272    8 -272   48  264 -104    8]
**********************************************************************
1 items had failures:
   1 of   8 in __main__.example_16
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/pcpa/.sage/tmp/fplll_17597.py
         [17.5 s]
 
----------------------------------------------------------------------
The following tests failed:


        sage -t  "devel/sage/sage/libs/fplll/fplll.pyx"
Total time for all tests: 17.6 seconds

Changed 9 years ago by pcpa

fplll patch

Changed 9 years ago by pcpa

sage patch

comment:6 Changed 9 years ago by fbissey

I must say I hate when I see that kind of sign flipping. Still we can have something based on something less ancient even if it is in compatibility mode. Now who could check that test before we also up the spkg.

Last edited 9 years ago by fbissey (previous) (diff)

comment:7 Changed 9 years ago by jpflori

  • Cc jpflori added

comment:8 Changed 8 years ago by Snark

Is it possible to avoid patching fplll? It it's not possible, did you discuss with upstream for inclusion?

comment:9 Changed 8 years ago by jpflori

I think upstream is more or less actively preparing a new release including fixes for Cygwin under my impulsion, so they should be quite reactive if you ask for the inclusion of such a patch as well.

comment:10 follow-up: Changed 8 years ago by Snark

  • Status changed from new to needs_info

I would like to see this go through : should I contact upstream myself?

comment:11 Changed 8 years ago by malb

I second that, I haven an interest in seeing this go in because I want to write libs.fplll4 which uses the new interface.

comment:12 in reply to: ↑ 10 Changed 8 years ago by jpflori

Replying to Snark:

I would like to see this go through : should I contact upstream myself?

Yup you should. I was in contact with Damien Stelhé around Christmas for #13354 and he included the fix I wanted in a to be released version. But when testing it I discovered other problems and got no reply so far. I must admit I did not ping him since the 30th of January...

So if you could also ask him for the last changes which were needed for Cygwin :)

comment:13 Changed 8 years ago by Snark

4.0.3 is out with cygwin changes ; it's already been seven weeks and I haven't written the mail I mentioned...

comment:14 Changed 8 years ago by jpflori

I just mailed Damien to thank him for the Cygwin changes and ask about our patch (from what I saw in the patch it seems not only legitimate, but needed to actually use the compatibility interface (except for the additional function we define of course)), so if you did not, please don't do it now.

comment:15 Changed 8 years ago by Snark

I had a mail discussion yesterday evening ; he is at a conference this week but will look into the matter later. :-P

Changed 8 years ago by jpflori

Changed 8 years ago by jpflori

comment:16 Changed 8 years ago by jpflori

  • Authors set to Paulo César Pereira de Andrade, Jean-Pierre Flori
  • Description modified (diff)
  • Status changed from needs_info to needs_review
  • Summary changed from upgrade fpLLL to newest upstream release to upgrade fpLLL to version 4.0.4

Some doctests changed but they are still LLL-reduced, so nothing is wrong.

Note: See TracTickets for help on using tickets.