Opened 7 years ago
Closed 6 years ago
#18437 closed task (fixed)
Switch from PolyBoRi to BRiAl
Reported by:  ohanar  Owned by:  

Priority:  major  Milestone:  sage6.9 
Component:  packages: standard  Keywords:  
Cc:  malb  Merged in:  
Authors:  R. Andrew Ohana  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  7ff3b09 (Commits, GitHub, GitLab)  Commit:  7ff3b098528643b5e7e698f9883a0a6b9107eb20 
Dependencies:  #19085  Stopgaps: 
Description (last modified by )
Right now PolyBoRi? relies on scons (which is still Python 2 only), and has some Python 2 specific syntax in its bindings. These issues will need to be resolved before Sage can support Python 3.
On this ticket we update to the first version of BRiAl which includes a new (incomplete, but sufficient for our purposes) autotools based system.
To achieve this, we fork upstream polybori
, see https://github.com/BRiAl/BRiAl
Tarball: https://github.com/BRiAl/BRiAl/releases/download/0.8.4.3/brial0.8.4.3.tar.bz2
Change History (85)
comment:1 Changed 7 years ago by
comment:2 followup: ↓ 4 Changed 7 years ago by
I think there are two separate issues:
1) polybori is using a internal copy of a third party package that may not be dead upstream (I think it has updates once in a while): cudd. 2) the python bindings
We could package the libraries properly and then have a separate python binding package, unless polybori
is altering its upstream significantly beyond the build system. The python binding could be moved in sage making it "a collective" responsibility.
comment:3 Changed 7 years ago by
Current version of cudd
in polybori
is 2.5.0 but upstream at http://vlsi.colorado.edu/~fabio/ has a version 2.5.1  it is somewhat alive.
comment:4 in reply to: ↑ 2 Changed 7 years ago by
Replying to fbissey:
I think there are two separate issues:
1) polybori is using a internal copy of a third party package that may not be dead upstream (I think it has updates once in a while): cudd. 2) the python bindings
We could package the libraries properly and then have a separate python binding package, unless
polybori
is altering its upstream significantly beyond the build system. The python binding could be moved in sage making it "a collective" responsibility.
Polybori has made some real modifications to cudd (I don't think that they are too significant though). From looking at the code it seems like they at least tried to support using an unmodified version, however I don't know if it actually works. Also, cudd's build system looks to be a joke (e.g. assumes sizeof(void *) == 4
unless you explicitly tell it otherwise), so I don't know how much I would trust it.
comment:5 Changed 7 years ago by
Doesn't sound good. Autotooling something like that is definitely a good idea. But that puts the burden of maintenance on us. Mind you in Gentoo we have patches to autotool all the individual components of suitesparse
independently of upstream, so it has been/is being done.
comment:6 Changed 7 years ago by
So I looked a bit more into how exactly polybori modifies cudd, and other than the build system changes, it appears to be just one of two things:
 Prefixing the symbols to not collide with another installation of cudd
 stripping out code that polybori doesn't use (there is only one case that polybori makes any meaningful changes to do this).
Given this, I don't think it would be hard to make polybori work with a vanilla version of cudd. Also, since upstream cudd is not completely dead, the author might be receptive to a better build system if we provide it.
comment:7 Changed 7 years ago by
 Branch set to u/ohanar/polybori
 Commit set to 8ee91768de785cbf3c0012a7b4a732d7b16a844b
 Status changed from new to needs_review
I contacted the author of Cudd on Tuesday and have yet to hear back from him. Given that, I think I would prefer not packaging Cudd separately, since maintaining one autotools based system (for polybori) is easier than two (for polybori and for cudd).
I've posted a branch, which uses the tarball at https://github.com/ohanar/PolyBoRi/releases/download/0.8.4/polybori0.8.4.tar.gz. It doesn't include any of the python bindings, nor any of the tests, so beyond the build system, it is a bit of a fork of polybori. (Maybe we should rename it to something like croppedpolybori.)
New commits:
083fabf  cleanup settings in polybori in module_list.py

f5b2b78  move polybori python interface into sage

8ee9176  use new autotools based polybori

comment:8 Changed 7 years ago by
I can easily test the new stuff in Gentoo. I can even test your stuff from git master if that helps.
comment:9 Changed 7 years ago by
 Commit changed from 8ee91768de785cbf3c0012a7b4a732d7b16a844b to b6aad4136aa9fc21844eaeacf11e9d32279f35c1
Branch pushed to git repo; I updated commit sha1. New commits:
b6aad41  use nonbroken version of polybori

comment:10 Changed 7 years ago by
Yes, I that would be good (at the very least to make sure that I haven't somehow made things work with old artifacts lying around). Point in fact, I just realized that I pushed a little prematurely, as I forgot to include a few headers in the dist. The new tarball is https://github.com/ohanar/PolyBoRi/releases/download/0.8.5/polybori0.8.5.tar.bz2.
I'm actually using the autotools branch, but at the moment the tip is the new release I just cut. Since this is new, I can squash the history at some point if desired.
comment:11 Changed 7 years ago by
 Description modified (diff)
comment:12 Changed 7 years ago by
Exactly what does this patch do? It looks like you're moving part of polybori
into the Sage library, why that?
comment:13 Changed 7 years ago by
 Status changed from needs_review to needs_info
comment:14 Changed 7 years ago by
Extraordinary measures (forking polybori???) need extraordinary justification...
comment:15 followup: ↓ 16 Changed 7 years ago by
Upstream being dead is ample justification. We had that conversation elsewhere and decided that it was the way to go.
comment:16 in reply to: ↑ 15 Changed 7 years ago by
comment:17 Changed 7 years ago by
We had some talk over at http://trac.sagemath.org/ticket/15530#comment:32 but I think the confirmation that Alexander and Michael had completely abandoned ship were posted on a thread on sagedevel. I'll dig that up.
comment:18 Changed 7 years ago by
From sagedevel "Python 3 focused Sage Days" thread: Hi, here's a reply from a PolyBoRi? developer:
Subject: Re: [Polyboridiscuss] Fwd: Re: [sagedevel] Re: Python 3 focused Sage Days Date: Saturday 10 Jan 2015, 22:19:16 From: Alexander Dreyer <adreyer@…> To: Martin Albrecht <martinralbrecht@…>, Polybori Discuss <polyboridiscuss@…>
Hi Martin, Unfortunately, Andrew's right, PolyBoRi? died when its developers left the scientific community. I'm not sure, what's the best way for SAGE to deal with it. Porting PolyBoRi?'s py files shouldn't be a big deal, and Sage already uses it's own Cythonbindings for the data structures. What remains is scons. Perhaps, it's easier to set up a setup.py or another build system than removing PolyBoRi? from all the crypto modules. (You probably know the dependencies better than me.) For working around lots of scons issues, most of the SConstruct file is plain python anyway.
Best regards,
Alexander
comment:19 Changed 7 years ago by
Even if upstream polybori
is dead and even if you fork it, that still doesn't mean it should be moved to the Sage library.
comment:20 followup: ↓ 21 Changed 7 years ago by
Fair point. The question then is "does anyone care to have to install sage to get polybori instead of having it as stand alone package?". If it has users as a stand alone package then it should be kept split. If not, that's at the discretion of whoever actually maintains the code.
comment:21 in reply to: ↑ 20 Changed 7 years ago by
Replying to fbissey:
Fair point. The question then is "does anyone care to have to install sage to get polybori instead of having it as stand alone package?".
I don't think that's "the question".
All the packaging effort in Sage is really converging towards separating packages from the Sage library proper.
At the very least, moving polybori
in the Sage library is a sufficiently big change that it should be discussed on sagedevel
.
comment:22 Changed 7 years ago by
If you really decide to move polybori
to the Sage library, then I think that
 moving
polybori
to the Sage library and  making it Python 3 compatible
should be two different tickets.
comment:23 Changed 7 years ago by
 Description modified (diff)
 Milestone changed from sage6.7 to sage6.8
 Summary changed from Fix PolyBoRi and Python 3 to Move PolyBoRi Python bindings to the Sage library and make it Python 3 compatible
comment:24 Changed 7 years ago by
And build/pkgs/polybori/SPKG.txt
needs to be updated.
comment:25 Changed 7 years ago by
 Summary changed from Move PolyBoRi Python bindings to the Sage library and make it Python 3 compatible to Move PolyBoRi Python bindings to the Sage library and remove PolyBoRi's dependency on scons
I haven't actually gone through the bindings and made sure they are python 3 compatible  I just fixed their doctests to work in the sage environment. It is easy enough for me to break out the new package into a different ticket.
I think we should rename the fork to say croppedpolybori, to avoid confusion.
comment:26 Changed 7 years ago by
 Branch changed from u/ohanar/polybori to u/ohanar/polybori_autotools
 Commit changed from b6aad4136aa9fc21844eaeacf11e9d32279f35c1 to 33e900c4ab03af3936f2b775495e3734e2b4ebb1
 Dependencies set to #18506
 Summary changed from Move PolyBoRi Python bindings to the Sage library and remove PolyBoRi's dependency on scons to Remove PolyBoRi's dependency on scons
comment:27 Changed 7 years ago by
 Commit changed from 33e900c4ab03af3936f2b775495e3734e2b4ebb1 to da490827368885a4370fe1f8352c07c107a938c4
comment:28 Changed 7 years ago by
 Commit changed from da490827368885a4370fe1f8352c07c107a938c4 to c30cf70daf27791feb128534cb1fad4d5b0dd652
Branch pushed to git repo; I updated commit sha1. New commits:
c30cf70  Revert "move polybori python interface into sage"

comment:29 Changed 7 years ago by
 Dependencies #18506 deleted
 Description modified (diff)
 Status changed from needs_info to needs_review
I updated the polybori autotools to include the python bindings.
comment:30 Changed 7 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_info
I think this needs more explanation/documentation. Surely build/pkgs/polybori/SPKG.txt
will need to be updated. In particular explain how you obtained the tarball.
comment:31 followup: ↓ 32 Changed 7 years ago by
Is this commit https://github.com/ohanar/PolyBoRi/commit/a91362c5be91b2fb23f8acf609852381b78b15a4 part of the tarball on this commit? I would prefer to see it as a patch in build/pkgs/polybori/patches
(and on a different ticket).
comment:32 in reply to: ↑ 31 ; followup: ↓ 34 Changed 7 years ago by
Replying to jdemeyer:
Is this commit https://github.com/ohanar/PolyBoRi/commit/a91362c5be91b2fb23f8acf609852381b78b15a4 part of the tarball on this commit?
Yes.
I would prefer to see it as a patch in
build/pkgs/polybori/patches
(and on a different ticket).
I would not. As I see it there are three routes we could go with polybori:
 We adopt the route that we took with Pynac (as a fork of Ginac). Our fork might in the future become tightly coupled with Sage, but we maintain it as a separate package outside of Sage.
 We make a minimal fork, only include the minimal changes necessary to build polybori without scons. If more substantive changes are needed, we include those in the Sage library.
 A hybrid approach: maintain a minimal fork, and a maintain a set of patches for more substantive changes.
I really dislike option 3 (which seems to be the one you are voting for). It seems like it just adds the unnecessary work of maintain patches, with no real benefit. Upon reflection, I would vote for 1, since it allows us make changes to the c++ source if we need to in the future.
If we go with option 1, I would rename this ticket to "Switch from polybori to XXXX", where XXXX is our fork of polybori (still needs a name, I haven't been 100% happy with anything I've come up with). In that case, I view it as perfectly acceptable for there to be additional changes to the fork, such as the Python 3 changes I made.
Also, @fbissey. The author of cudd got back to me finally, he said he was already planning on making a new release sometime this summer which uses autotools. Until then, I think I'll continue bundling it with polybori.
comment:33 Changed 7 years ago by
There is also the issue of not doing too much on a single ticket. If you make a ticket which is only about compiling polybori with autotools, I'll be happy to review it. Anything more will only make the review process more difficult.
comment:34 in reply to: ↑ 32 Changed 7 years ago by
Replying to ohanar:
 We adopt the route that we took with Pynac (as a fork of Ginac). Our fork might in the future become tightly coupled with Sage, but we maintain it as a separate package outside of Sage.
One thing I don't like is that this will mean essentially unreviewed code in Sage.
From a practical point of view, there is not much difference between code in the Sage library and code in some upstream project which is only used in Sage.
But the code in the Sage library is supposed to be reviewed, while code in upstream projects is normally not reviewed (if people are using upstream outside of Sage, that means at least that the code is tested).
To be honest, I also don't have an optimal solution. Maybe that's why you should ask for feedback on sagedevel
.
comment:35 Changed 7 years ago by
Posted on sagedevel for feed back: https://groups.google.com/forum/#!topic/sagedevel/9Uhnvm3wef8
comment:36 Changed 7 years ago by
 Cc malb added
comment:37 Changed 6 years ago by
 Branch changed from u/ohanar/polybori_autotools to u/ohanar/brial
 Commit changed from c30cf70daf27791feb128534cb1fad4d5b0dd652 to fdff73302359b87ab524b5a7f3722e36a550ca7f
 Description modified (diff)
 Status changed from needs_info to needs_review
 Summary changed from Remove PolyBoRi's dependency on scons to Switch from PolyBoRi to BRiAl
I finally got back to this. This switches us to BRiAl (the successor/fork to polybori, as discussed in the sagedevel thread), and includes the new autotools based build system. I will repurpose #18506. for the second version of BRiAl (to be released), which will include some python 3 syntax fixes.
New commits:
72965f6  Merge branch 'polybori_autotools' into HEAD

fdff733  switch to BRiAl

comment:38 followup: ↓ 40 Changed 6 years ago by
Cool to see some move on this! spkginstall
still has messages about polybori
, it is especially important to update those not too confuse newcomers.
I see that the library name hasn't changed upstream (libpolybori*
), that would be something to consider although it is not a show stopper for this.
I notice ipython
has been dropped from the list of dependencies, is it on purpose?
comment:39 followup: ↓ 41 Changed 6 years ago by
Do I understand well that libgd
is looked for if libpng
is not there? It is all a bit strange. If m4ri
doesn't have support for png
I guess have_libpng
will be empty and then libgd
will be checked. The line
AC_DEFINE([PBORI_HAVE_GD],[],[has gd png support])
is a bit misleading as if you arrive there, you have libgd
support which may or may not have png
included in it.
Also why shouldn't the compilation abort if m4ri_png=yes
but you cannot find libpng
? As far as I can tell m4ri
won't use libgd
for libpng unless you abuse the configuration options, so that whole structure looks very strange.
I guess on closer look I have an issue with the way m4ri/m4rie/polybory deal with some of their optional dependencies, and that may be something to look into in the future. For starter if m4ri
is compiled with png
support, the used libpng
should be included in output of pkgconfig m4ri libs
.
comment:40 in reply to: ↑ 38 Changed 6 years ago by
Replying to fbissey:
Cool to see some move on this!
spkginstall
still has messages aboutpolybori
, it is especially important to update those not too confuse newcomers.
Sure, I'll do that shortly.
I see that the library name hasn't changed upstream (
libpolybori*
), that would be something to consider although it is not a show stopper for this.
I didn't want to have to deal with this on the sage side of things at the moment, so I didn't change this. There are a ton of places that the polybori name is used throughout the source code, and it would be a pain to change them, although changing the library name and headers shouldn't be too bad.
I notice
ipython
has been dropped from the list of dependencies, is it on purpose?
It shouldn't have been a dependency in the first place from what I can tell.
comment:41 in reply to: ↑ 39 Changed 6 years ago by
Replying to fbissey:
I basically just took the sconstruct logic and moved it into configure.ac (with the appropriate language translation). I don't claim it is good logic.
comment:42 Changed 6 years ago by
 Commit changed from fdff73302359b87ab524b5a7f3722e36a550ca7f to 5cbf983ee15dd61d8ad4aec707b58e743def3c72
Branch pushed to git repo; I updated commit sha1. New commits:
5cbf983  rename error messages PolyBoRi > BRiAl

comment:43 Changed 6 years ago by
comment:44 Changed 6 years ago by
I just greped the sage source code for traces of polybori
import. Since it now installs in brial
unless I missed a mechanism the followings need to be updated
Mirage:sage fbissey$ grep r "from polybori" * rings/polynomial/multi_polynomial_sequence.py: from polybori import gauss_on_polys rings/polynomial/multi_polynomial_sequence.py: from polybori.ll import eliminate,ll_encode,ll_red_nf_redsb rings/polynomial/multi_polynomial_sequence.py: from polybori.interred import interred as inter_red rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: #from polybori.interpolate import interpolate_smallest_lex rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid, BooleanMonomial rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid, BooleanMonomial rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import BooleSet rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomial rings/polynomial/pbori.pyx: from polybori.parallel import _encode_polynomial rings/polynomial/pbori.pyx: sage: from polybori import BooleSet rings/polynomial/pbori.pyx: from polybori import red_tail rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: from polybori.gbcore import groebner_basis rings/polynomial/pbori.pyx: from polybori import red_tail rings/polynomial/pbori.pyx: sage: from polybori import BooleSet rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import BooleSet rings/polynomial/pbori.pyx: sage: from polybori import BooleSet rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import groebner_basis rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import BooleanPolynomialVector rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import groebner_basis rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import GroebnerStrategy rings/polynomial/pbori.pyx: sage: from polybori import BooleanMonomialMonoid rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import map_every_x_to_x_plus_one rings/polynomial/pbori.pyx: sage: from polybori import zeros rings/polynomial/pbori.pyx: sage: from polybori.interpolate import * rings/polynomial/pbori.pyx: sage: from polybori.interpolate import * rings/polynomial/pbori.pyx: sage: from polybori import ll_red_nf_redsb rings/polynomial/pbori.pyx: sage: from polybori import ll_red_nf_noredsb rings/polynomial/pbori.pyx: sage: from polybori import ll_red_nf_noredsb_single_recursive_call rings/polynomial/pbori.pyx: sage: from polybori import if_then_else rings/polynomial/pbori.pyx: sage: from polybori import top_index rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import substitute_variables rings/polynomial/pbori.pyx: sage: from polybori import substitute_variables rings/polynomial/pbori.pyx: sage: from polybori import random_set, set_random_seed rings/polynomial/pbori.pyx: sage: from polybori import random_set, set_random_seed rings/polynomial/pbori.pyx:# todo: merge with pickling from polybori.parallel rings/polynomial/pbori.pyx:# todo: merge with pickling from polybori.parallel rings/polynomial/pbori.pyx: from polybori.parallel import _decode_polynomial rings/polynomial/pbori.pyx:# todo: merge with pickling from polybori.parallel rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import BooleConstant rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * rings/polynomial/pbori.pyx: sage: from polybori import * sat/converters/__init__.py:from polybori import CNFEncoder as PolyBoRiCNFEncoder
comment:45 Changed 6 years ago by
 Commit changed from 5cbf983ee15dd61d8ad4aec707b58e743def3c72 to 51a1e98e18acc7f127ef6a5f9a0f0993c867f800
Branch pushed to git repo; I updated commit sha1. New commits:
51a1e98  fix imports polybori > brial

comment:46 Changed 6 years ago by
 Description modified (diff)
New commits:
51a1e98  fix imports polybori > brial

comment:47 Changed 6 years ago by
One last thing. I think we should implement a removal of old polybori
install like in the old polybori
spkg and while we are at it cleaning of a previous brial
install in case of updating or reinstall. We should do it before the call to configure
.
comment:48 Changed 6 years ago by
 Commit changed from 51a1e98e18acc7f127ef6a5f9a0f0993c867f800 to bf3c56f1ca605b04ff60e0ab9f72c19158c6f9d6
Branch pushed to git repo; I updated commit sha1. New commits:
bf3c56f  brial: clean out old polybori and brial installations before installing

comment:49 Changed 6 years ago by
I think it is ready for testing. One more thing that I just spotted
libraries=['polybori', 'polybori_groebner', 'm4ri', 'gd', 'png12'],
shouldn't include gd
we have removed the dependency in polybori and a check in sageongentoo (all compiled with Wl,asneeded
) shows that it is not used in pbori.so
, there is only one instance of a direct linking to libgd
in the whole sage, I'll check where it is coming from.
comment:50 Changed 6 years ago by
matrix_mod2_dense
is where there is a direct linking to gd
and it is explicit in module_list.py
. The other 2 instances sage.modules.vector_mod2_dense
and sage.rings.polynomial.pbori
(whose libraries
's line was the object of my previous comment) are probably linked to the presence of m4ri
and can be removed. Actually the presence in matrix_mod2_dense is curious and should be further investigated but that's not this ticket.
comment:51 Changed 6 years ago by
 Commit changed from bf3c56f1ca605b04ff60e0ab9f72c19158c6f9d6 to 7aadcf2a49b065b56cb737e325c5e47d01c4a710
Branch pushed to git repo; I updated commit sha1. New commits:
7aadcf2  remove unused library gd from pbori

comment:52 Changed 6 years ago by
Ok just tested a build from scratch with the branch. The python bindings are being installed in SAGE_LOCAL/lib64/python2.7/sitepackages
that lib64
is a bit of a problem. We'll have to dig in the configuration script as libpolybori*
is installed correctly under SAGE_LOCAL/lib
.
comment:53 Changed 6 years ago by
Adding
AC_SUBST(pythondir)
in configure.ac
(yes in lower case see http://www.gnu.org/software/automake/manual/html_node/Python.html) seem to have solved the problem. That or my autotools did a better job. I am successfully building the doc now, which is where things fell apart the first time.
comment:54 followup: ↓ 61 Changed 6 years ago by
Amusing, polybori
was the last dependency pulling scons
. Consequently my build from scratch doesn't include scons
and the associated tests are failing. If we have an associated ticket for scons
demotion to optional or removal it will depend on this one and viceversa.
sage t long warnlong 116.1 src/sage/tests/cmdline.py ********************************************************************** File "src/sage/tests/cmdline.py", line 578, in sage.tests.cmdline.test_executable Failed example: out.find("SCons") >= 0 Expected: True Got: False ********************************************************************** File "src/sage/tests/cmdline.py", line 580, in sage.tests.cmdline.test_executable Failed example: err Expected: '' Got: 'Traceback (most recent call last):\n File "/usr/lib/pythonexec/python2.7/scons", line 188, in <module>\n import SCons.Script\nImportError: No module named SCons.Script\n' ********************************************************************** File "src/sage/tests/cmdline.py", line 582, in sage.tests.cmdline.test_executable Failed example: ret Expected: 0 Got: 1 **********************************************************************
comment:55 followup: ↓ 57 Changed 6 years ago by
OK I have failing tests because cnf.py
is not installed. It is the only one missing, I am guessing this is because it is not listed in pyroot/Makefile.am
as I see it on github. However it is listed in the tarball you are linking from github. How did this happen? I cannot see a commit touching pyroot/Makefile.am
other than the one moving it in pyroot
.
comment:56 Changed 6 years ago by
And it wasn't included in the tarball for 0.8.4.
comment:57 in reply to: ↑ 55 Changed 6 years ago by
Replying to fbissey:
OK I have failing tests because
cnf.py
is not installed. It is the only one missing, I am guessing this is because it is not listed inpyroot/Makefile.am
as I see it on github. However it is listed in the tarball you are linking from github. How did this happen? I cannot see a commit touchingpyroot/Makefile.am
other than the one moving it inpyroot
.
Yes, I fixed that for 0.8.4.1, look at the tag on github. I probably had a staging commit that didn't include cnf.py
, and somehow that is on the master branch.
comment:58 Changed 6 years ago by
OK see it under the tag but of course when I worked on the lib/lib64 I was working from master.
comment:59 followup: ↓ 62 Changed 6 years ago by
scons was already demoted to optional, but since we still had standard packages depending on it, it was in practice standard so these doctest errors weren't caught.
I fixed master with a force push (to reflect the correct commit). If you want to send a pull request fixing the lib64/lib issue, please do. I don't see it on OS X (obviously) and I don't have access to a decent linux box at the moment either.
comment:60 Changed 6 years ago by
Will send the pull request in a few hours when I can properly focus again.
comment:61 in reply to: ↑ 54 Changed 6 years ago by
Replying to fbissey:
sage t long warnlong 116.1 src/sage/tests/cmdline.py ********************************************************************** File "src/sage/tests/cmdline.py", line 578, in sage.tests.cmdline.test_executable Failed example: out.find("SCons") >= 0 Expected: True Got: False ********************************************************************** File "src/sage/tests/cmdline.py", line 580, in sage.tests.cmdline.test_executable Failed example: err Expected: '' Got: 'Traceback (most recent call last):\n File "/usr/lib/pythonexec/python2.7/scons", line 188, in <module>\n import SCons.Script\nImportError: No module named SCons.Script\n' ********************************************************************** File "src/sage/tests/cmdline.py", line 582, in sage.tests.cmdline.test_executable Failed example: ret Expected: 0 Got: 1 **********************************************************************
positive_review to a new ticket which just removes those 3 tests.
comment:62 in reply to: ↑ 59 Changed 6 years ago by
Replying to ohanar:
I fixed master with a force push (to reflect the correct commit). If you want to send a pull request fixing the lib64/lib issue, please do. I don't see it on OS X (obviously) and I don't have access to a decent linux box at the moment either.
Done. I may just take care of Jeroen's request  I will take it literally.
comment:63 Changed 6 years ago by
 Dependencies set to 19085
#19085 filled and we should depend on it.
comment:64 Changed 6 years ago by
 Dependencies changed from 19085 to #19085
 Milestone changed from sage6.8 to sage6.9
comment:65 Changed 6 years ago by
 Commit changed from 7aadcf2a49b065b56cb737e325c5e47d01c4a710 to 2a799b99c7aeec58e7f88a8210322ed25aa97edc
Branch pushed to git repo; I updated commit sha1. New commits:
2a799b9  BRiAl: minor version bump with sitepackages fix

comment:66 Changed 6 years ago by
 Description modified (diff)
comment:67 Changed 6 years ago by
Building from scratch on a clean machine...
comment:68 Changed 6 years ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
I think we are ready to go!
comment:69 Changed 6 years ago by
 Status changed from positive_review to needs_work
comment:70 Changed 6 years ago by
I'm guessing this issue comes down to not including the SIMD cflags of m4ri. I can't seem to figure out how to extract these from <m4ri/m4ri_config.h>
using autotools. @fbissey any ideas?
comment:71 Changed 6 years ago by
If that's the case, yes I do have an idea. But I'll look at the log more in depth first. It looks like missing sse2 instruction but the build is i486 so it shouldn't have them either.
comment:72 Changed 6 years ago by
I think I know what to do, expect a pull request shortly.
comment:73 Changed 6 years ago by
Here is what I do in M4RIE
https://bitbucket.org/malb/m4rie/src/HEAD/m4/ax_m4ri_flags.m4?at=master
but I think this should be replaced by pkgconfig like this
comment:74 Changed 6 years ago by
Funny, I wrote what I called a vile hack but at some point it was looking very very close to that. I just didn't have the step to use cat
afterwards. For some reason I didn't want to go that way.
comment:75 Changed 6 years ago by
 Commit changed from 2a799b99c7aeec58e7f88a8210322ed25aa97edc to 61c0186f4e4467c7bff459781bfaa831c9a9dc53
Branch pushed to git repo; I updated commit sha1. New commits:
61c0186  brial: version bump which includes M4RI's SIMD CFLAGS

comment:76 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
ok, hopefully now fixed
comment:77 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:78 followup: ↓ 79 Changed 6 years ago by
 Status changed from positive_review to needs_work
Isn't this a typo?
+ $(INST)/$(BRAIL) \
comment:79 in reply to: ↑ 78 ; followup: ↓ 80 Changed 6 years ago by
Replying to jdemeyer:
Isn't this a typo?
+ $(INST)/$(BRAIL) \
Looks like one. I wonder how I got it to build "without that problem" on two different machines.
comment:80 in reply to: ↑ 79 Changed 6 years ago by
Replying to fbissey:
Looks like one. I wonder how I got it to build "without that problem" on two different machines.
$(INST)/$(BRAIL)
becomes $(INST)/
which is an existing directory...
The problem would start if you upgrade $(BRIAL)
, then the Sage library would not be automatically rebuilt.
comment:81 Changed 6 years ago by
Here is a build failure because the dependencies are not correct, looks like the Sage library is built before brial:
comment:82 Changed 6 years ago by
 Description modified (diff)
comment:83 Changed 6 years ago by
 Commit changed from 61c0186f4e4467c7bff459781bfaa831c9a9dc53 to 7ff3b098528643b5e7e698f9883a0a6b9107eb20
Branch pushed to git repo; I updated commit sha1. New commits:
7ff3b09  brial: fix typo in deps

comment:84 Changed 6 years ago by
 Status changed from needs_work to positive_review
OK, positive review  take 3.
comment:85 Changed 6 years ago by
 Branch changed from u/ohanar/brial to 7ff3b098528643b5e7e698f9883a0a6b9107eb20
 Resolution set to fixed
 Status changed from positive_review to closed
Reposted from #15530:
fyi, I started working on an autotools based build system for polybori at https://github.com/ohanar/PolyBoRi/tree/autotools. I'll probably work on it a bit more next weekend during SD 64.25, but the important bits (the c++ libraries) are currently working with only a couple of hacks. What remains (for sage at least) is just a little cleanup and throwing in the bit of the python bindings we use.
I also don't know how we should handle this with regards to upstream, since upstream is dead. I certainly don't want to take over maintenance for polybori, but we'll probably need to make some sort of fork.