Opened 5 years ago

Closed 5 years ago

#18437 closed task (fixed)

Switch from PolyBoRi to BRiAl

Reported by: ohanar Owned by:
Priority: major Milestone: sage-6.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) Commit: 7ff3b098528643b5e7e698f9883a0a6b9107eb20
Dependencies: #19085 Stopgaps:

Description (last modified by vbraun)

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/brial-0.8.4.3.tar.bz2

Change History (85)

comment:1 Changed 5 years ago by ohanar

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.

comment:2 follow-up: Changed 5 years ago by 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.

comment:3 Changed 5 years ago by fbissey

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 5 years ago by ohanar

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.

Last edited 5 years ago by ohanar (previous) (diff)

comment:5 Changed 5 years ago by fbissey

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 5 years ago by ohanar

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:

  1. Prefixing the symbols to not collide with another installation of cudd
  2. 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 5 years ago by ohanar

  • Authors set to R. Andrew Ohana
  • 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/polybori-0.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 cropped-polybori.)


New commits:

083fabfcleanup settings in polybori in module_list.py
f5b2b78move polybori python interface into sage
8ee9176use new autotools based polybori
Last edited 5 years ago by ohanar (previous) (diff)

comment:8 Changed 5 years ago by fbissey

I can easily test the new stuff in Gentoo. I can even test your stuff from git master if that helps.

comment:9 Changed 5 years ago by git

  • Commit changed from 8ee91768de785cbf3c0012a7b4a732d7b16a844b to b6aad4136aa9fc21844eaeacf11e9d32279f35c1

Branch pushed to git repo; I updated commit sha1. New commits:

b6aad41use non-broken version of polybori

comment:10 Changed 5 years ago by ohanar

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/polybori-0.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 5 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 5 years ago by jdemeyer

Exactly what does this patch do? It looks like you're moving part of polybori into the Sage library, why that?

Last edited 5 years ago by jdemeyer (previous) (diff)

comment:13 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_info

comment:14 Changed 5 years ago by jdemeyer

Extraordinary measures (forking polybori???) need extraordinary justification...

comment:15 follow-up: Changed 5 years ago by fbissey

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 5 years ago by jdemeyer

Replying to fbissey:

We had that conversation elsewhere

Pointers please...

comment:17 Changed 5 years ago by fbissey

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 sage-devel. I'll dig that up.

comment:18 Changed 5 years ago by fbissey

From sage-devel "Python 3 focused Sage Days" thread: Hi, here's a reply from a PolyBoRi? developer:


Subject: Re: [Polybori-discuss] Fwd: Re: [sage-devel] Re: Python 3 focused Sage Days Date: Saturday 10 Jan 2015, 22:19:16 From: Alexander Dreyer <adreyer@…> To: Martin Albrecht <martinralbrecht@…>, Polybori Discuss <polybori-discuss@…>

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 Cython-bindings 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 5 years ago by jdemeyer

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 follow-up: Changed 5 years ago by 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?". 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 5 years ago by jdemeyer

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 sage-devel.

comment:22 Changed 5 years ago by jdemeyer

If you really decide to move polybori to the Sage library, then I think that

  1. moving polybori to the Sage library and
  2. making it Python 3 compatible

should be two different tickets.

comment:23 Changed 5 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-6.7 to sage-6.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 5 years ago by jdemeyer

And build/pkgs/polybori/SPKG.txt needs to be updated.

comment:25 Changed 5 years ago by ohanar

  • 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 cropped-polybori, to avoid confusion.

comment:26 Changed 5 years ago by ohanar

  • 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

I broke off the bindings portion to #18506.


New commits:

a05ce2dmove polybori python interface into sage
cdca588cleanup settings in polybori in module_list.py
33e900cuse new autotools based polybori

comment:27 Changed 5 years ago by git

  • Commit changed from 33e900c4ab03af3936f2b775495e3734e2b4ebb1 to da490827368885a4370fe1f8352c07c107a938c4

Branch pushed to git repo; I updated commit sha1. New commits:

60725e3Merge remote-tracking branch 'trac/develop' into polybori_autotools
da49082update to a polybori which includes python bindings

comment:28 Changed 5 years ago by git

  • Commit changed from da490827368885a4370fe1f8352c07c107a938c4 to c30cf70daf27791feb128534cb1fad4d5b0dd652

Branch pushed to git repo; I updated commit sha1. New commits:

c30cf70Revert "move polybori python interface into sage"

comment:29 Changed 5 years ago by ohanar

  • 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 5 years ago by jdemeyer

  • 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 follow-up: Changed 5 years ago by jdemeyer

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 ; follow-up: Changed 5 years ago by ohanar

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:

  1. 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.
  1. 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.
  1. 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 5 years ago by jdemeyer

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 5 years ago by jdemeyer

Replying to ohanar:

  1. 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 sage-devel.

comment:35 Changed 5 years ago by fbissey

comment:36 Changed 5 years ago by malb

  • Cc malb added

comment:37 Changed 5 years ago by ohanar

  • 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 sage-devel 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:

72965f6Merge branch 'polybori_autotools' into HEAD
fdff733switch to BRiAl

comment:38 follow-up: Changed 5 years ago by fbissey

Cool to see some move on this! spkg-install still has messages about polybori, it is especially important to update those not too confuse new-comers.

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?

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

comment:39 follow-up: Changed 5 years ago by fbissey

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 pkg-config m4ri --libs.

comment:40 in reply to: ↑ 38 Changed 5 years ago by ohanar

Replying to fbissey:

Cool to see some move on this! spkg-install still has messages about polybori, it is especially important to update those not too confuse new-comers.

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 5 years ago by ohanar

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 5 years ago by git

  • Commit changed from fdff73302359b87ab524b5a7f3722e36a550ca7f to 5cbf983ee15dd61d8ad4aec707b58e743def3c72

Branch pushed to git repo; I updated commit sha1. New commits:

5cbf983rename error messages PolyBoRi -> BRiAl

comment:43 Changed 5 years ago by fbissey

I say we save my last comment for future work because I think m4ri configure script needs surgery first.


New commits:

5cbf983rename error messages PolyBoRi -> BRiAl

New commits:

5cbf983rename error messages PolyBoRi -> BRiAl

comment:44 Changed 5 years ago by fbissey

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 5 years ago by git

  • Commit changed from 5cbf983ee15dd61d8ad4aec707b58e743def3c72 to 51a1e98e18acc7f127ef6a5f9a0f0993c867f800

Branch pushed to git repo; I updated commit sha1. New commits:

51a1e98fix imports polybori -> brial

comment:46 Changed 5 years ago by ohanar

  • Description modified (diff)

New commits:

51a1e98fix imports polybori -> brial

comment:47 Changed 5 years ago by fbissey

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 re-install. We should do it before the call to configure.

comment:48 Changed 5 years ago by git

  • Commit changed from 51a1e98e18acc7f127ef6a5f9a0f0993c867f800 to bf3c56f1ca605b04ff60e0ab9f72c19158c6f9d6

Branch pushed to git repo; I updated commit sha1. New commits:

bf3c56fbrial: clean out old polybori and brial installations before installing

comment:49 Changed 5 years ago by fbissey

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 sage-on-gentoo (all compiled with -Wl,-as-needed) 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 5 years ago by fbissey

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 5 years ago by git

  • Commit changed from bf3c56f1ca605b04ff60e0ab9f72c19158c6f9d6 to 7aadcf2a49b065b56cb737e325c5e47d01c4a710

Branch pushed to git repo; I updated commit sha1. New commits:

7aadcf2remove unused library gd from pbori

comment:52 Changed 5 years ago by fbissey

Ok just tested a build from scratch with the branch. The python bindings are being installed in SAGE_LOCAL/lib64/python2.7/site-packages 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 5 years ago by fbissey

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 follow-up: Changed 5 years ago by fbissey

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 vice-versa.

sage -t --long --warn-long 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/python-exec/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 follow-up: Changed 5 years ago by 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 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 5 years ago by fbissey

And it wasn't included in the tarball for 0.8.4.

comment:57 in reply to: ↑ 55 Changed 5 years ago by ohanar

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 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.

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 5 years ago by fbissey

OK see it under the tag but of course when I worked on the lib/lib64 I was working from master.

comment:59 follow-up: Changed 5 years ago by ohanar

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 5 years ago by fbissey

Will send the pull request in a few hours when I can properly focus again.

comment:61 in reply to: ↑ 54 Changed 5 years ago by jdemeyer

Replying to fbissey:

sage -t --long --warn-long 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/python-exec/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 5 years ago by fbissey

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 5 years ago by fbissey

  • Dependencies set to 19085

#19085 filled and we should depend on it.

comment:64 Changed 5 years ago by fbissey

  • Dependencies changed from 19085 to #19085
  • Milestone changed from sage-6.8 to sage-6.9

comment:65 Changed 5 years ago by git

  • Commit changed from 7aadcf2a49b065b56cb737e325c5e47d01c4a710 to 2a799b99c7aeec58e7f88a8210322ed25aa97edc

Branch pushed to git repo; I updated commit sha1. New commits:

2a799b9BRiAl: minor version bump with site-packages fix

comment:66 Changed 5 years ago by ohanar

  • Description modified (diff)

comment:67 Changed 5 years ago by fbissey

Building from scratch on a clean machine...

comment:68 Changed 5 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

I think we are ready to go!

comment:70 Changed 5 years ago by ohanar

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 5 years ago by fbissey

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 5 years ago by fbissey

I think I know what to do, expect a pull request shortly.

comment:73 Changed 5 years ago by malb

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 pkg-config like this

https://autotools.io/pkgconfig/pkg_check_modules.html

comment:74 Changed 5 years ago by fbissey

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 5 years ago by git

  • Commit changed from 2a799b99c7aeec58e7f88a8210322ed25aa97edc to 61c0186f4e4467c7bff459781bfaa831c9a9dc53

Branch pushed to git repo; I updated commit sha1. New commits:

61c0186brial: version bump which includes M4RI's SIMD CFLAGS

comment:76 Changed 5 years ago by ohanar

  • Description modified (diff)
  • Status changed from needs_work to needs_review

ok, hopefully now fixed

comment:77 Changed 5 years ago by fbissey

  • Status changed from needs_review to positive_review

comment:78 follow-up: Changed 5 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Isn't this a typo?

+ $(INST)/$(BRAIL) \

comment:79 in reply to: ↑ 78 ; follow-up: Changed 5 years ago by fbissey

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 5 years ago by jdemeyer

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 5 years ago by vbraun

Here is a build failure because the dependencies are not correct, looks like the Sage library is built before brial:

http://build.sagemath.org/release/builders/%20%20slow%20Oxford%20ARM%20%28Ubuntu%2012.10%29%20incremental/builds/182/steps/compile/logs/stdio

comment:82 Changed 5 years ago by vbraun

  • Description modified (diff)

comment:83 Changed 5 years ago by git

  • Commit changed from 61c0186f4e4467c7bff459781bfaa831c9a9dc53 to 7ff3b098528643b5e7e698f9883a0a6b9107eb20

Branch pushed to git repo; I updated commit sha1. New commits:

7ff3b09brial: fix typo in deps

comment:84 Changed 5 years ago by fbissey

  • Status changed from needs_work to positive_review

OK, positive review - take 3.

comment:85 Changed 5 years ago by vbraun

  • Branch changed from u/ohanar/brial to 7ff3b098528643b5e7e698f9883a0a6b9107eb20
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.