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

add experimental libFES package #13162

Closed
sagetrac-Bouillaguet mannequin opened this issue Jun 25, 2012 · 48 comments
Closed

add experimental libFES package #13162

sagetrac-Bouillaguet mannequin opened this issue Jun 25, 2012 · 48 comments

Comments

@sagetrac-Bouillaguet
Copy link
Mannequin

sagetrac-Bouillaguet mannequin commented Jun 25, 2012

libfes is a library (under development) that solves systems of boolean equations by exhaustive search. It is dramatically faster than Groebner bases in general.

Installation Guide

sage: from sage.libs.fes import exhaustive_search
sage: R.<x,y,z> = BooleanPolynomialRing()
sage: f = [x*y + z + y + 1, x+y+z, x*y + y*z]
sage: ideal(f).variety()
sage: exhaustive_search(f)

Note to release manage the SPKG should be added to the experimental repository, but the patch should be applied to Sage.

Depends on #13202

CC: @malb

Component: packages: experimental

Author: Charles Bouillaguet

Reviewer: Martin Albrecht

Merged: sage-5.7.beta1

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

@sagetrac-Bouillaguet sagetrac-Bouillaguet mannequin added this to the sage-5.2 milestone Jun 25, 2012
@sagetrac-Bouillaguet

This comment has been minimized.

@sagetrac-Bouillaguet

This comment has been minimized.

@malb
Copy link
Member

malb commented Jun 25, 2012

Reviewer: Martin Albrecht

@malb
Copy link
Member

malb commented Jun 25, 2012

comment:4

Hi, I found a few small issues (so far) :)

SPKG

  • little error in SPKG.txt
  === mypackage-0.1 (Charles Bouillaguet, 25 June 2012) ===
  • hg status in the SPKG reports files that are not checked in.
  • please include the Trac ticket number in the SPKG.txt changelog (and the hg commit message of your SPKG)

Patch

  • since FES is a library its interface should live in sage.libs.fes and not sage.rings.polynomial.fes
  • exhaustive_search has neither documentation nor doctests

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jun 28, 2012

comment:6

patch and spkg updated

@malb
Copy link
Member

malb commented Jun 28, 2012

comment:7

Hi, almost there IMHO:

  • the docstring isn't properly formated:
    • INPUT shouldn't get a double ":" but a single one
    • AFAIK the argument list after input should look like
INPUT:

 - ``foo`` - description
  • There shouldn't be leading "#" in your comments between code blocks, indentation does take care of it
  • sage: blocks should be indented by four spaces wrt to the preceding block ending with :: this includes the comments in between these code blocks
  • who frees dense_rep_eqs?

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jun 29, 2012

comment:8

Hi,

patch modified again (use the second one, not the first one). I updated the docstrings according to the standard. Also, NOW, dense_rep_eqs is freed :)

@malb
Copy link
Member

malb commented Jul 2, 2012

comment:10

SPKG

  • This looks bad:
wrapper.c: In function ‘exhaustive_search_wrapper’:
wrapper.c:17:5: warning: implicit declaration of function ‘exhaustive_ia32’ [-Wimplicit-function-declaration]
wrapper.c:23:5: warning: implicit declaration of function ‘exhaustive_simd’ [-Wimplicit-function-declaration]
  • You should add "-fPIC" to CFLAGS in spkg-install, it won't compile without it (on my machine)

Patch

  • Your patch was incorrectly indented, I fixed this in the updated patch attached to this ticket.

@malb

This comment has been minimized.

@sagetrac-Bouillaguet

This comment has been minimized.

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jul 2, 2012

comment:13

spkg updated (-fPIC and no compilation warnings)

@malb
Copy link
Member

malb commented Jul 3, 2012

comment:15

I still get:

main_simd.c: In function ‘main’:
main_simd.c:58:3: warning: implicit declaration of function ‘exhaustive_simd’ [-Wimplicit-function-declaration]

Btw. is there an upstream repository/website?

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jul 3, 2012

comment:16

Replying to @malb:

Btw. is there an upstream repository/website?

https://bitbucket.org/fes/fes

It's fresh :)

@malb
Copy link
Member

malb commented Jul 4, 2012

comment:17

And it's private, i.e., I cannot access it :)

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jul 4, 2012

comment:18

Replying to @malb:

And it's private, i.e., I cannot access it :)

fixed

@malb
Copy link
Member

malb commented Jul 6, 2012

comment:19

Hi, can you reproduce the error I am seeing (above)?

@jdemeyer
Copy link

jdemeyer commented Jan 8, 2013

comment:38

There are uncommitted changes in the spkg:

jdemeyer@sage:~/spkg/fes-0.1$ hg status
M spkg-install

@jdemeyer
Copy link

jdemeyer commented Jan 8, 2013

comment:39

The following can be removed from spkg-install:

unset RM

INCLUDES="-I$SAGE_LOCAL/include"
LIBDIRS="-L$SAGE_LOCAL/lib"

And if $MAKE check actually does something, it should be inside spkg-check and not in spkg-install.

@jdemeyer
Copy link

jdemeyer commented Jan 8, 2013

comment:41

All doctests in fes.pyx need to be marked

# optional - fes

such that doctests succeed when libFES is not installed.

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jan 17, 2013

comment:42

I updated the spkg (taking into account Jeroen's comments).

I don't think that marking the doctests as optional is necessary, as the .pyx file is NOT included in the source tree when the spkg is not installed (cf. magic in module_list.py).

However, the more I think about this, the more I am convinced that the right way to access this library from sage is through #13850. In that case, the doctest involving libFES in the documentation of .solve() would need to be marked optional, of course.

@sagetrac-Bouillaguet

This comment has been minimized.

@jdemeyer
Copy link

comment:43

Replying to @sagetrac-Bouillaguet:

I updated the spkg (taking into account Jeroen's comments).

I don't think that marking the doctests as optional is necessary, as the .pyx file is NOT included in the source tree when the spkg is not installed (cf. magic in module_list.py).

That's not true. The module_list.py magic is only about compiling the Cython file, which is completely orthogonal to doctests. The fes.pyx file is always there (regardless of whether it is compiled), so the doctests are there.

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jan 17, 2013

Changed dependencies from #13202 to #13202,#13964

@sagetrac-Bouillaguet sagetrac-Bouillaguet mannequin modified the milestones: sage-5.6, sage-5.7 Jan 17, 2013
@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jan 18, 2013

new patch, because the library interface has changed

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jan 18, 2013

Changed dependencies from #13202,#13964 to #13202

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jan 18, 2013

comment:45

Attachment: fes.2.patch.gz

Hi malb & Jeroen. OK, I get it now, sorry for the noise. I marked the doctests as optional and fixed the spkg.

Since I have installed the libFES package in my sage tree, I don't see how I could test without it? (how to uninstall a spkg ?).

@malb
Copy link
Member

malb commented Jan 20, 2013

comment:46

I don't think you can (well, you can delete the files by hand of course).

@sagetrac-Bouillaguet
Copy link
Mannequin Author

sagetrac-Bouillaguet mannequin commented Jan 23, 2013

comment:47

@malb : if you don't have the libFES spkg installed, can you check that this patch applies and passes the tests ?

@malb
Copy link
Member

malb commented Jan 23, 2013

comment:48

Yep, all tests pass. Sorry for the delay!

@haraldschilly
Copy link
Member

comment:49

spkg is on the servers ...

@jdemeyer
Copy link

Merged: sage-5.7.beta1

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

3 participants