Opened 8 years ago

Closed 7 years ago

#11575 closed enhancement (fixed)

Update PolyBoRi to release 0.8.0

Reported by: AlexanderDreyer Owned by: AlexanderDreyer
Priority: major Milestone: sage-5.0
Component: packages: standard Keywords:
Cc: AlexanderDreyer, PolyBoRi, fbissey, burcin, malb, SimonKing, jdemeyer Merged in: sage-5.0.beta1
Authors: Alexander Dreyer Reviewers: Martin Albrecht
Report Upstream: None of the above - read trac for reasoning. Work issues:
Branch: Commit:
Dependencies: #11574, #9138 Stopgaps:

Description (last modified by jdemeyer)

PolyBoRi 0.8 will come with some changes in the interface (in particular the infamous global settings will be removed)

So there was an extensive alpha phase this time: https://sourceforge.net/projects/polybori/files/polybori/0.8.alphas/

The final release is here: https://sourceforge.net/projects/polybori/files/polybori/0.8.0/

Current patches/spkg

Attachments (13)

polybori-0.8.0alpha.p1.patch (59.0 KB) - added by AlexanderDreyer 8 years ago.
First working patch for supporting PolyBoRi? 0.8 by the Sage-lib
polybori-9999-20110725.log.bz2 (11.8 KB) - added by fbissey 8 years ago.
build log from OS X linking fails when making shared libraries badly.
polybori-0.8.0alpha.p2-against-p1.patch (6.6 KB) - added by AlexanderDreyer 8 years ago.
Fix to allowd to use PolyBoRI 0.8 alpha1
polybori-0.8.0alpha.p3.patch (92.7 KB) - added by AlexanderDreyer 8 years ago.
Patch which uses a wrapper for c++ classes without default constructor
polybori-0.8-rc.p0.patch (101.0 KB) - added by AlexanderDreyer 8 years ago.
Cummulative patch for interfacing with PolyBoRi? 0.8-rc1
polybori-0.8-rc.p1VSp0.patch (2.1 KB) - added by AlexanderDreyer 8 years ago.
Patch for using polybori-0.8-rc.p1.spkg (against polybori-0.8-rc.p0.patch)
polybori-0.8-rc.p1.patch (102.4 KB) - added by AlexanderDreyer 8 years ago.
Cummulative patch, rebased to Sage 4.7.1
polybori-0.8-rc.p2.patch (102.4 KB) - added by AlexanderDreyer 8 years ago.
most recent cummulative patch
polybori-0.8-rc.p3.patch (102.5 KB) - added by AlexanderDreyer 8 years ago.
Rebased patch to 4.7.2alpha1
polybori-0.8-rc-11574.patch (1.9 KB) - added by AlexanderDreyer 8 years ago.
Additional patch if #11574 is merged
polybori-0.8.0.p0.patch (103.6 KB) - added by AlexanderDreyer 8 years ago.
Patch for integration PolyBoRi?'s release 0.8.0 (depends on #11574)
polybori-0.8.0.p1.patch (97.3 KB) - added by AlexanderDreyer 7 years ago.
Patch rebased to Sage 4.7.2 alpha3 (+ deps), fixed cygwin 0.15 issue
polybori-0.8.0.p1.2.patch (97.4 KB) - added by jdemeyer 7 years ago.
Rebased to latest #9138

Download all attachments as: .zip

Change History (161)

comment:1 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

comment:2 Changed 8 years ago by fbissey

For the record I have a gentoo ebuild that pulls the trunk of hg on bitbucket. That's how I tested things in the issues I have filled for polybori recently.

comment:3 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

I prepared a first at http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8-alpha.p0.spkg

It builds, but of course sage -br and tests will fail, since the Sage library needs to be adjusted.

comment:4 Changed 8 years ago by fbissey

Build using my ebuild (you didn't commit anything since you tagged 0.8-alpha so that's what I should have pulled). Used boost-1.42 and m4ri-20100817. Builds and install fine without warnings. Is there a working test suite that can be run? Also I didn't build any docs.

comment:5 Changed 8 years ago by AlexanderDreyer

Hi Francois! I gave to access to polybori-testsuite. If you do use it via the cython interface from Sage's Library (*not* the original PyPolyBoRi?,so). I need to update this, but it's not that easy, because I have to c++-ify some classes without defautl constructor.

comment:6 Changed 8 years ago by fbissey

Hi Alexander, thanks for giving me access. I think it is best to test it separate from sage at first, especially when you consider that installing it will break sage. I hope that it can work that way.

comment:7 follow-up: Changed 8 years ago by fbissey

I just had a thought. I should really try this against m4ri-20110715 (#11574). I suppose this version of polybori is ready for it.

comment:8 in reply to: ↑ 7 ; follow-up: Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

I just had a thought. I should really try this against m4ri-20110715 (#11574). I suppose this version of polybori is ready for it.

It should, but I haven't had time for testing the most recent version yet.

comment:9 in reply to: ↑ 8 Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

Replying to fbissey:

I just had a thought. I should really try this against m4ri-20110715 (#11574). I suppose this version of polybori is ready for it.

It should, but I haven't had time for testing the most recent version yet.

I'll have to wait until the sage server are fully back on, m4ri tarball seem to be stored there. That or I clone m4ri tip from bitbucket, which may be an idea since I already to that for polybori.

comment:10 Changed 8 years ago by fbissey

Pulled m4ri tip. Polybori-0.7.1 compiles against it, no problem. I'll check 0.8.alpha a bit later and then I'll see about that test suite.

Changed 8 years ago by AlexanderDreyer

First working patch for supporting PolyBoRi? 0.8 by the Sage-lib

comment:11 Changed 8 years ago by AlexanderDreyer

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

Here's now the first patch for making the Sage-lib supporting PolyBoRi? 0.8: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11575/polybori-0.8.0alpha.p1.patch

I needs to workaround missing default generators of PolyBoRi?-functions by reintroducing then (just for now: It should be using the cppclass construct) . Therefore, we do need another patched spkg: http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8-alpha.p1.spkg

comment:12 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

comment:13 follow-up: Changed 8 years ago by fbissey

I have done a build of your latest tip from a couple of days ago and it was fine. I still have to get organized to do the testing. Nice to have a patch for sage but I am afraid it will need to be rebased now that #11377 is in. Please do not use SAGE_ROOT in module_list.py. In your case SAGE_INC will be enough.

comment:14 in reply to: ↑ 13 ; follow-up: Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

I have done a build of your latest tip from a couple of days ago and it was fine. I still have to get organized to do the testing. Nice to have a patch for sage but I am afraid it will need to be rebased now that #11377 is in. Please do not use SAGE_ROOT in module_list.py. In your case SAGE_INC will be enough.

No problem, it's still work to do anyway, since the c++-ification not really done yet. (I just mis-clicked, it should read "NEEDS_WORKS".)

PS: So that crash-issue on gentoo-prefix got away with 0.8-alpha?

comment:15 in reply to: ↑ 14 ; follow-up: Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

Replying to fbissey:

I have done a build of your latest tip from a couple of days ago and it was fine. I still have to get organized to do the testing. Nice to have a patch for sage but I am afraid it will need to be rebased now that #11377 is in. Please do not use SAGE_ROOT in module_list.py. In your case SAGE_INC will be enough.

No problem, it's still work to do anyway, since the c++-ification not really done yet. (I just mis-clicked, it should read "NEEDS_WORKS".)

PS: So that crash-issue on gentoo-prefix got away with 0.8-alpha?

Don't know yet. I have to build this on OS X and apply the patch. It may very well not happen until Monday local time (it's Saturday morning here) since this is one of my work machine.

comment:16 in reply to: ↑ 15 ; follow-up: Changed 8 years ago by AlexanderDreyer

Don't know yet. I have to build this on OS X and apply the patch. It may very well not happen until Monday local time (it's Saturday morning here) since this is one of my work machine.

Here, too (1:00).

In case this still fails: Can you give the plain PolyBoRi? source from bitbucket a try? I had to patch the recent spkg to work around some cython vs. c++ issue.

comment:17 Changed 8 years ago by AlexanderDreyer

  • Owner changed from tbd to AlexanderDreyer

comment:18 in reply to: ↑ 16 Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

Don't know yet. I have to build this on OS X and apply the patch. It may very well not happen until Monday local time (it's Saturday morning here) since this is one of my work machine.

Here, too (1:00).

In case this still fails: Can you give the plain PolyBoRi? source from bitbucket a try? I had to patch the recent spkg to work around some cython vs. c++ issue.

Not quite lunch time here. I'll make a note to try both. The nature of sage-on-gentoo is that the plain sources will be used over spkg unless the spkg is the only thing that exists or works. We still patch stuff if it is not too big (OK sometimes even when it is big).

comment:19 Changed 8 years ago by fbissey

Pulled the latest mercurial tip. Installed fine on linux. OS X (10.5.8 on x86) didn't like it one bit. I compiled with -ggdb which may have an impact.

i686-apple-darwin9-g++ -o groebner/libpolybori_groebner.dylib -Wl,-dead_strip_dylibs -dynamiclib groebner/src/groebner.os groebner/s
rc/LiteralFactorization.os groebner/src/LiteralFactorizationIterator.os groebner/src/randomset.os groebner/src/pairs.os groebner/src
/groebner_alg.os groebner/src/FGLMStrategy.os groebner/src/polynomial_properties.os groebner/src/LexBucket.os groebner/src/dlex4data
.os groebner/src/dp_asc4data.os groebner/src/lp4data.os groebner/src/nf.os groebner/src/interpolate.os groebner/src/GroebnerStrategy
.os groebner/src/PairManager.os groebner/src/PolyEntry.os groebner/src/ReductionStrategy.os groebner/src/MatrixMonomialOrderTables.o
s -L/Users/frb15/Desktop/Gentoo/usr/lib -L/Users/frb15/Desktop/Gentoo/usr/lib/python2.7/config -Lpolybori -Lgroebner -LCudd -lm -lgd
 -lgd -lm4ri -lgd -lgd -lm4ri
ld: warning: directory 'polybori' following -L not found

That's the critical bit where it fails. I'll compare with what happens on linux shortly.

Francois who is a bit snowed in today (some more falling).

comment:20 follow-up: Changed 8 years ago by AlexanderDreyer

  • Status changed from needs_review to needs_work

@fbissey Please pull again. There was indeed an outdated directory name given, which was ignored on Linux.

comment:21 in reply to: ↑ 20 Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

@fbissey Please pull again. There was indeed an outdated directory name given, which was ignored on Linux.

Still some problem making dylib (which in fact we are not using on OS X at the moment) if I read the message properly. I will attach a build log shortly. Will take a few minutes since it is on a remote machine.

Changed 8 years ago by fbissey

build log from OS X linking fails when making shared libraries badly.

comment:22 Changed 8 years ago by fbissey

Build log attached. Linking of the dylib is what fails in a spectacular manner. gcc is 4.2.1, apple sources.

comment:23 follow-up: Changed 8 years ago by AlexanderDreyer

The unresolved references are from libpolybori. If seems, that the dynlibs have to resolve all Symbols (unlike '.so's on Linux). I've put another patch to bitbucket, which should cure this issue.

comment:24 in reply to: ↑ 23 Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

The unresolved references are from libpolybori. If seems, that the dynlibs have to resolve all Symbols (unlike '.so's on Linux). I've put another patch to bitbucket, which should cure this issue.

No effect at all. I have the same output (except for the modification you made on the last linking line).

comment:25 Changed 8 years ago by fbissey

  • Owner changed from AlexanderDreyer to fbissey

we may have to consider the possibility that libpolybori/libpolybori.dylib is broken. That would explain why trying to use shared object from polybori doesn't work in sage.

comment:26 Changed 8 years ago by fbissey

Could you add the following linker option for shared libraries on OS X:

-Wl,-undefined -Wl,dynamic_lookup

That may help.

comment:27 Changed 8 years ago by fbissey

I should add that it seem to help linking that last library here.

comment:28 Changed 8 years ago by AlexanderDreyer

I added this to the SConstruct, you may pull from bitbucket again. Unfortunately, I do not have access to any OSX machine, so I cannot try.

If there are more command line flags to add, you may temporarily given them to scons by adding a file called custom.py in the main directory. For instance, for this case custom.py is as follows:

SHLINKFLAGS+="-Wl,-undefined -Wl,dynamic_lookup"

(scons -h given you the names of the possible variables.)

comment:29 Changed 8 years ago by fbissey

Your new SConstruct is there in what I pulled but it appeared not to have made it to the linking line. I am investigating this more closely in case there is something in the set up on my side. Hopefully I'll solve this and deliver a solution to you quickly (but may be not today in your time zone).

comment:30 follow-up: Changed 8 years ago by AlexanderDreyer

Maybe, the loadable module is just wrong here. I removed it (temporarily?), see the newest push at bitbucket.

comment:31 in reply to: ↑ 30 ; follow-up: Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

Maybe, the loadable module is just wrong here. I removed it (temporarily?), see the newest push at bitbucket.

No effects from that change BUT I was passing

SHLINKFLAGS="${LDFLAGS} -dynamiclib"

to scons which suppressed the flags you set. From the look of things I think it should be good now on OS X. If you had the flag appended instead of just set it may have worked earlier, not sure how you are supposed to do that in scons....

comment:32 in reply to: ↑ 31 Changed 8 years ago by AlexanderDreyer

No effects from that change BUT I was passing

SHLINKFLAGS="${LDFLAGS} -dynamiclib"

to scons which suppressed the flags you set. From the look of things I think it should be good now on OS X. If you had the flag appended instead of just set it may have worked earlier, not sure how you are supposed to do that in scons....

Thank you very much or this hint! I changed the SConstruct file to append the respective stuff. I hope this will work now without manual settings.

comment:33 Changed 8 years ago by AlexanderDreyer

There a new alpha at https://sourceforge.net/projects/polybori/files/polybori/0.8.alphas/ and at bitbucket. Many thanks for all bug hunters! I hope I can provide a new spkg soon.

comment:34 Changed 8 years ago by PolyBoRi

The latest spkg builds fine on Mac OS X 10.6. However I using the patch I get:

sage/rings/polynomial/pbori.cpp: In function ‘PyObject* __pyx_pf_4sage_5rings_10polynomial_5pbori_21BooleanPolynomialRing_30set(PyObject*, PyObject*)’:
sage/rings/polynomial/pbori.cpp:12802: error: ‘class polybori::BoolePolyRing’ has no member named ‘activate’
/Applications/sage/local/include/csage/ccobject.h: In function ‘T* Construct_ppp(void*, const P&, const Q&, const R&) [with T = polybori::BoolePolyRing, P = long unsigned int, Q = int, R = bool]’:
sage/rings/polynomial/pbori.cpp:4976:   instantiated from here
/Applications/sage/local/include/csage/ccobject.h:59: error: no matching function for call to ‘polybori::BoolePolyRing::BoolePolyRing(const long unsigned int&, const int&, const bool&)’
/Applications/sage/local/include/polybori/BoolePolyRing.h:123: note: candidates are: polybori::BoolePolyRing::BoolePolyRing()
/Applications/sage/local/include/polybori/BoolePolyRing.h:120: note:                 polybori::BoolePolyRing::BoolePolyRing(const polybori::BoolePolyRing&)
/Applications/sage/local/include/polybori/BoolePolyRing.h:116: note:                 polybori::BoolePolyRing::BoolePolyRing(size_t, const boost::shared_ptr<polybori::COrderingBase>&)
/Applications/sage/local/include/polybori/BoolePolyRing.h:113: note:                 polybori::BoolePolyRing::BoolePolyRing(size_t, int)
/Applications/sage/local/include/polybori/BoolePolyRing.h:107: note:                 polybori::BoolePolyRing::BoolePolyRing(const polybori::CWeakPtr<polybori::CCuddCore>&)
/Applications/sage/local/include/polybori/BoolePolyRing.h:104: note:                 polybori::BoolePolyRing::BoolePolyRing(const boost::intrusive_ptr<polybori::CCuddCore>&)
error: command 'gcc' failed with exit status 1
sage: There was an error installing modified sage library code.

Cheers, Michael

comment:35 Changed 8 years ago by AlexanderDreyer

At least pbori.cpp is not updated. Did you qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11575/polybori-0.8.0alpha.p1.patch to the Sage library and qpush?

Maybe you alos have to touch pbori.pyx before sage -b.

Changed 8 years ago by AlexanderDreyer

Fix to allowd to use PolyBoRI 0.8 alpha1

comment:36 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

Hi Michael, you're right, I changed to many things in alpha1. Please import the second patch also. I also had to do a minor change in the spkg. Unfortunately, I cannot upload the updated spkg currently due to some server problems at boxen.

Regards,

Alexander

comment:37 Changed 8 years ago by AlexanderDreyer

The updates spkg is now at the same place as before: http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8-alpha.p2.spkg

Changed 8 years ago by AlexanderDreyer

Patch which uses a wrapper for c++ classes without default constructor

comment:38 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)
  • Owner changed from fbissey to AlexanderDreyer

I've added now a (cumulative) patch: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11575/polybori-0.8.0alpha.p3.patch It includes both previous ones as well as the treatment of c++ classes, that so not have a default constructor. (The latter should better to be handled by cython directly, so I'll report that issue there.)

The spkg was also updated (dummy default constructors dropped): http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8-alpha.p3.spkg

The sources are in sync with the original repository now. If this version is fine for everybody, I'll bundle the first release candidate for polybori 0.8 soon.

Note, that there are some todos due (see above).

PS: I'm taking ownership of this ticket again.

Changed 8 years ago by AlexanderDreyer

Cummulative patch for interfacing with PolyBoRi? 0.8-rc1

comment:39 Changed 8 years ago by AlexanderDreyer

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

Meanwhile, we bundled PolyBoRi 0.8-rc1 upstream.

Here is also the updated spkg: http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8-rc.p0.spkg and the Sage patch for interfacing with PolyBoRi 0.8-rc1: http://trac.sagemath.org/sage_trac/attachment/ticket/11575/polybori-0.8-rc.p0.patch

I rebased on the most recent version of the spkg from #11261 (still not official).

comment:40 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)
  • Owner changed from AlexanderDreyer to (none)

comment:41 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

You'll find an updated spkg at http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8-rc.p1.spkg which contains the sources of PolyBoRi 0.8 rc2. The Sage library patch stays the same.

comment:42 Changed 8 years ago by AlexanderDreyer

The most recent spkg builds, installs and tests sage/rings/polynomial successfully on !OpenSuSE 11.1 and !MacOS 10.6. It is not affected by #11681 any more. Once Sage 4.7.1 is through the mirrors. I'll rebase the Sage-lib patch.

Changed 8 years ago by AlexanderDreyer

Patch for using polybori-0.8-rc.p1.spkg (against polybori-0.8-rc.p0.patch)

comment:43 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

Sorry, I forgot to upload the following patch: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11575/polybori-0.8-rc.p1VSp0.patch Please apply both in a row.

comment:44 follow-up: Changed 8 years ago by fbissey

Now that 4.7.1 is officially released polybori-0.8-rc.p0.patch really needs rebasing.

Changed 8 years ago by AlexanderDreyer

Cummulative patch, rebased to Sage 4.7.1

comment:45 in reply to: ↑ 44 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

Replying to fbissey:

Now that 4.7.1 is officially released polybori-0.8-rc.p0.patch really needs rebasing.

Yes, indeed. It's here now: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11575/polybori-0.8-rc.p1.patch I obeyed the changes around SAGE_INC as you pointed out.

Since #11261 is merged now, I'm going to rebase the spkg, too.

comment:46 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

comment:47 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

Now the rebased spkg (on #11261) here: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11575/polybori-0.8-rc.p2.patch It also includes the updated sources of http://sourceforge.net/projects/polybori/files/polybori/0.8.alphas/polybori-0.8.0rc3.tar.gz (minor changes). It's tested with Sage 4.7.1 on SLES 11.

comment:48 follow-up: Changed 8 years ago by fbissey

OK I pulled the trunk again with my ebuild and after a few changes to accommodate for your last commit (lib/lib64 issue not really essential for sage as far as I can see) it installed fine on linux-amd64.

Trying on OS X (10.5.8) turned something interesting which may explain some of the difficulties with dynamic linking on OS X.

 *   correcting install_name of libpolybori.dylib ...                                                                        [ ok ]
 *   correcting install_name of libpolybori_groebner.dylib ...                                                               [ ok ]
>>> Completed installing polybori-9999 into /Users/frb15/Desktop/Gentoo/var/tmp/portage/sci-mathematics/polybori-9999/image/Users/frb15/Desktop/Gentoo/

ecompressdir: bzip2 -9 /Users/frb15/Desktop/Gentoo/usr/share/man
 * QA Notice: invalid reference to libpolybori/libpolybori.dylib in /Users/frb15/Desktop/Gentoo/usr/lib/libpolybori_groebner.dylib
 * ERROR: sci-mathematics/polybori-9999 failed:
 *   invalid install_name found, your application or library will crash at runtime

I am guessing the culprit line is this one

i686-apple-darwin9-g++ -o groebner/libpolybori_groebner.dylib -Wl,-dead_strip_dylibs -dynamiclib groebner/src/groebner.os groebner/src/LiteralFactorization.os groebner/src/LiteralFactorizationIterator.os groebner/src/randomset.os groebner/src/pairs.os groebner/src/groebner_alg.os groebner/src/FGLMStrategy.os groebner/src/polynomial_properties.os groebner/src/LexBucket.os groebner/src/dlex4data.os groebner/src/dp_asc4data.os groebner/src/lp4data.os groebner/src/nf.os groebner/src/interpolate.os groebner/src/GroebnerStrategy.os groebner/src/PairManager.os groebner/src/PolyEntry.os groebner/src/ReductionStrategy.os groebner/src/MatrixMonomialOrderTables.os -Llibpolybori -Lgroebner -L/Users/frb15/Desktop/Gentoo/usr/lib -L/Users/frb15/Desktop/Gentoo/usr/lib/python2.7/config -LCudd -lm -lboost_unit_test_framework -lm4ri -lm4ri libpolybori/libpolybori.dylib -lgd

Because you already have "-Llibpolybori" you shouldn't need "libpolybori/libpolybori.dylib" you should just plain link with "-lpolybori".

comment:49 in reply to: ↑ 48 Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

Because you already have "-Llibpolybori" you shouldn't need "libpolybori/libpolybori.dylib" you should just plain link with "-lpolybori".

Thanks, I fixed that on bitbucket.

comment:50 follow-up: Changed 8 years ago by fbissey

Thanks for doing that but I unfortunately still get the same message. I am investigating what needs to be done. I am hoping there will be an easy fix.

comment:51 in reply to: ↑ 50 Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

Thanks for doing that but I unfortunately still get the same message. I am investigating what needs to be done. I am hoping there will be an easy fix.

That's weired. I had the chance to test it on an Intel-Mac with OS X 10.6 and an ppc running 10.5. In both cases, the build succeeded. (But not using prefix though.)

comment:52 follow-up: Changed 8 years ago by AlexanderDreyer

PS: Does the linking sequence still include libpolybori/libpolybori.dylib ?

comment:53 in reply to: ↑ 52 Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

PS: Does the linking sequence still include libpolybori/libpolybori.dylib ?

No you fixed that exactly as I asked you. Portage does extra checks for consistency on libraries on OS X. There are a number of things that aren't exactly common knowledge to people who don't do a lot of development on OS X. Technically all libraries on OS X have an rpath which usually is relative. Look at this document: http://blog.onesadcookie.com/2008/01/installname-magic.html The solution adopted in portage in a OS X prefix is to make the install_name absolute which is why at the moment a sage-prefix on OS X is not relocatable. Now what the QA tool of portage is telling us is that libpolybori_groebner.dylib is linked to libpolybori/libpolybori.dylib as in the library contains the name and the path of the library. And we of course don't want that.

Try the following command:

otool -L libpolybori_groebner.dylib

The result here is as follow:

otool -L libpolybori_groebner.dylib 
libpolybori_groebner.dylib:
	/Users/frb15/Desktop/Gentoo/usr/lib/libpolybori_groebner.dylib (compatibility version 0.0.0, current version 0.0.0)
	libpolybori/libpolybori.dylib (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 111.1.7)
	/Users/frb15/Desktop/Gentoo/usr/lib/libm4ri-0.0.20100817.dylib (compatibility version 0.0.0, current version 0.0.0)
	/Users/frb15/Desktop/Gentoo/usr/lib/libgd.2.dylib (compatibility version 3.0.0, current version 3.0.0)
	/Users/frb15/Desktop/Gentoo/usr/lib/gcc/i686-apple-darwin9/4.2.1/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.9.0)
	/Users/frb15/Desktop/Gentoo/usr/lib/gcc/i686-apple-darwin9/4.2.1/libgcc_s.1.dylib (compatibility version 1.0.0, current version 1.0.0)

you can notice libpolybori/libpolybori.dylib on its lonesome.

comment:54 Changed 8 years ago by fbissey

I am guessing that the fundamental problem is that the install_name of libpolybori.dylib is set to libpolybori/libpolybori.dylib and I only change it after install. So when it is linked that's what is used. I can change it to something correct with install_name_tool -change but it would be better if we had something consistent in the first place. The man page for ld on OS X

     -install_name name
                 Sets an internal "install path" (LC_ID_DYLIB) in a dynamic
                 library. Any clients linked against the library will record
                 that path as the way dyld should locate this library.  If
                 this option is not specified, then the -o path will be used.
                 This option is also called -dylib_install_name for compati‐
                 bility.

We could try to use -flat_namespace which basically gives you the same kind of situation as linux regarding the loading of libraries.

comment:55 follow-up: Changed 8 years ago by fbissey

make that -Wl,-flat_namespace, it is to be passed to ld.

comment:56 in reply to: ↑ 55 ; follow-up: Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

make that -Wl,-flat_namespace, it is to be passed to ld.

I added this to the bitbucket repo.

comment:57 in reply to: ↑ 56 ; follow-up: Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

Replying to fbissey:

make that -Wl,-flat_namespace, it is to be passed to ld.

I added this to the bitbucket repo.

I cannot test it until Monday unfortunately. Have you looked at what "otool -L" reports for you after adding that?

comment:58 in reply to: ↑ 57 Changed 8 years ago by AlexanderDreyer

for some reason, it still owns paths:

sage subshell) p900sys02:sage dreyer$ otool -L local/lib/libpolybori_groebner.dylib 
local/lib/libpolybori_groebner.dylib:
	groebner/libpolybori_groebner-0.8.0.dylib.0.0.0 (compatibility version 0.0.0, current version 0.0.0)
	libpolybori/libpolybori-0.8.0.dylib.0.0.0 (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 125.2.0)
	/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib/libm4ri-0.0.20100701.dylib (compatibility version 0.0.0, current version 0.0.0)
	/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib/libgd.2.dylib (compatibility version 3.0.0, current version 3.0.0)
	/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib/libpng12.0.dylib (compatibility version 36.0.0, current version 36.0.0)
	/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.5)
	/usr/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.9.0)

This is how it was build:

g++ -o groebner/libpolybori_groebner-0.8.0.dylib.0.0.0 -m64 -dynamiclib -Wl,-flat_namespace groebner/src/groebner.os groebner/src/LiteralFactorization.os groebner/src/LiteralFactorizationIterator.os groebner/src/randomset.os groebner/src/pairs.os groebner/src/groebner_alg.os groebner/src/FGLMStrategy.os groebner/src/polynomial_properties.os groebner/src/LexBucket.os groebner/src/dlex4data.os groebner/src/dp_asc4data.os groebner/src/lp4data.os groebner/src/nf.os groebner/src/interpolate.os groebner/src/GroebnerStrategy.os groebner/src/PairManager.os groebner/src/PolyEntry.os groebner/src/ReductionStrategy.os groebner/src/MatrixMonomialOrderTables.os -Llibpolybori -Lgroebner -L/Users/dreyer/sage/local/lib -L/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib -L/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib/python2.6/config -LCudd -lpolybori -lm -lm4ri -lm4ri -lgd -lpng12 -lz

and

g++ -o libpolybori/libpolybori-0.8.0.dylib.0.0.0 -m64 -dynamiclib -Wl,-flat_namespace Cudd/cudd/cuddAPI.os Cudd/cudd/cuddCache.os Cudd/cudd/cuddInit.os Cudd/cudd/cuddLCache.os Cudd/cudd/cuddRef.os Cudd/cudd/cuddTable.os Cudd/cudd/cuddZddFuncs.os Cudd/cudd/cuddZddSetop.os libpolybori/src/BoolePolyRing.os libpolybori/src/BooleEnv.os libpolybori/src/BoolePolynomial.os libpolybori/src/BooleVariable.os libpolybori/src/CCheckedIdx.os libpolybori/src/CErrorInfo.os libpolybori/src/PBoRiError.os libpolybori/src/CCuddFirstIter.os libpolybori/src/BooleMonomial.os libpolybori/src/BooleSet.os libpolybori/src/LexOrder.os libpolybori/src/CCuddLastIter.os libpolybori/src/BooleExponent.os libpolybori/src/DegLexOrder.os libpolybori/src/DegRevLexAscOrder.os libpolybori/src/pbori_routines.os libpolybori/src/BlockDegLexOrder.os libpolybori/src/BlockDegRevLexAscOrder.os -Llibpolybori -Lgroebner -L/Users/dreyer/sage/local/lib -L/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib -L/Users/buildbot/build/sage/bsd-1/bsd_64_binary/build/sage-4.7/local/lib/python2.6/config -LCudd -lm -lm4ri -lm4ri

comment:59 Changed 8 years ago by fbissey

I will have to look a bit more deeply into it. There may be some extra sauce needed. I'll look at what libtool does.

comment:60 follow-up: Changed 8 years ago by AlexanderDreyer

Ok, it seems, it has takes libpolybori/libpolybori-0.8.0.dylib.0.0.0 from the -o option, so I'll have to build stuff in one single directory. (This will need some time)

comment:61 in reply to: ↑ 60 ; follow-up: Changed 8 years ago by fbissey

Replying to AlexanderDreyer:

Ok, it seems, it has takes libpolybori/libpolybori-0.8.0.dylib.0.0.0 from the -o option, so I'll have to build stuff in one single directory. (This will need some time)

That certainly something you could do, but I was hoping that it wouldn't be necessary.

comment:62 in reply to: ↑ 61 Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

That certainly something you could do, but I was hoping that it wouldn't be necessary.

Ok, I changed this, let's see on Monday (I do not have access to a Mac either).

comment:63 Changed 8 years ago by AlexanderDreyer

I manager to install gentoo-prefix on an old ppc to debug things further. The most current polybori builds nicely on that prefix, but I can now reproduce issue mentioned here: https://bitbucket.org/brickenstein/polybori/issue/1/non-aligned-pointer-being-freed-on-os-x when using wither ipbori or invoking ipython with the following manual commands: Python 2.7.2 (default, Aug 22 2011, 13:36:51) Type "copyright", "credits" or "license" for more information.

IPython 0.10.2 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object'. ?object also works, ?? prints more.

In [1]: import sys

In [2]: sys.path.insert(0,"pyroot")

In [3]: from polybori.frontend import *  
[...]
python2.7(77363) malloc: *** error for object 0x1d315a8: Non-aligned pointer being freed
*** set a breakpoint in malloc_error_break to debug
python2.7(77363) malloc: *** mmap(size=3221204992) failed (error code=12)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
[...]
*** set a breakpoint in malloc_error_break to debug
python2.7(77388) malloc: *** mmap(size=3221204992) failed (error code=12)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug

In [4]: x(1)*x(1)
python2.7(77388) malloc: *** error for object 0x1d315a8: Non-aligned pointer being freed
*** set a breakpoint in malloc_error_break to debug
Out[4]: x(

Invoking plain python doesn't cause any trouble:

Python 2.7.2 (default, Aug 22 2011, 13:36:51)
[GCC 4.2.1 (Gentoo 4.2.1_p5666, Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path.insert(0,"pyroot")
>>> from polybori.frontend import *
>>> x(1)*x(1)
x(1)

So maybe an ipython issue?

comment:64 Changed 8 years ago by fbissey

I am checking but I think that may be a verbosity issue. python-2.7 is not very verbose and while the warnings look terrible it never prevented sage to start or even prevented me to run the tests.

comment:65 Changed 8 years ago by AlexanderDreyer

Ok, if I build everything but the initialization as shared lib and link the remaining small bundle to these libs it does work. I'll investigate that further.

comment:66 follow-up: Changed 8 years ago by fbissey

I am getting a bus error with ipython and the latest trunk (with some manual massaging to get what looks like correct libraries) and no error with pure python that's indeed very strange. The backtrace I put on bitbucket was pointing to polybori but there may be more to it.

Looking back in this thread what happened to -Wl,-undefined -Wl,dynamic_lookup for OS X? I suspect that would do the trick we are after. I looked at what libtool does in the case of m4ri:

gcc -dynamiclib -Wl,-undefined -Wl,dynamic_lookup -o .libs/libm4ri-0.0.20100701.dylib  .libs/brilliantrussian.o .libs/misc.o .libs/packedmatrix.o .libs/grayflex.o .libs/strassen.o .libs/permutation.o .libs/trsm.o .libs/pls.o .libs/solve.o .libs/pls_mmpf.o .libs/echelonform.o    -mmmx -msse -msse2 -msse3   -install_name  /Users/frb15/Desktop/sage-4.7.1/local/lib/libm4ri-0.0.20100701.dylib  -Wl,-single_module

This is from a vanilla (ie not gentoo based) build log. I detect relocation problems on OS X.

comment:67 Changed 8 years ago by AlexanderDreyer

BTW: how do you fix relative pathes to the libs to absolute ones (without setting DYLD_LIBRARY_PATH)?

comment:68 Changed 8 years ago by fbissey

In the case of m4ri libtool is doing the guessing. For polybori I do things manually:

install_name_tool -id ${absolutepath} ${library}

And for libpolybori_groebner

install_name_tool -change libpolybori.dylib ${absolutepathoflibpolybori.dylib} libpolybori_groebner.dylib

comment:69 follow-up: Changed 8 years ago by fbissey

Do you know how to run ipython under gdb?

comment:70 in reply to: ↑ 69 Changed 8 years ago by AlexanderDreyer

  • Status changed from needs_review to needs_work

Replying to fbissey:

Do you know how to run ipython under gdb?

Unfortunately not. But one should be able to find this in the sage -gdb script.

Thanks for pointing to install_name_tool

comment:71 follow-up: Changed 8 years ago by fbissey

that helped. Does this make any sense to you

ython2.7(21230) malloc: *** error for object 0x249ae10: Non-aligned pointer being freed (2)
*** set a breakpoint in malloc_error_break to debug

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x0000005c
0x90b39afa in std::__numpunct_cache<char>::_M_cache ()
(gdb) bt
#0  0x90b39afa in std::__numpunct_cache<char>::_M_cache ()
#1  0x90b41feb in std::__write<char> ()
#2  0x90b3c408 in std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<long> ()
#3  0x90b3c54f in std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put ()
#4  0x02446f7a in std::ostream::_M_insert<long> ()
#5  0x0166438e in polybori::CCuddCore::CCuddCore ()
#6  0x01661680 in polybori::BoolePolyRing::BoolePolyRing ()
#7  0x015a3d52 in init_module_PyPolyBoRi ()
#8  0x021780fd in boost::python::handle_exception_impl ()
#9  0x0217917c in boost::python::handle_exception<void (*)()> ()
#10 0x02178e83 in boost::python::detail::init_module ()
#11 0x015a3c2b in initPyPolyBoRi ()
#12 0x00269231 in _PyImport_LoadDynamicModule ()

comment:72 in reply to: ↑ 66 Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

Looking back in this thread what happened to -Wl,-undefined -Wl,dynamic_lookup for OS X? I suspect that would do the trick we are after. I looked at what libtool does in the case of m4ri:

I misunderstood that this was superseeded by -Wl,-flat_namespace . Should this also be added to the bundle?

comment:73 in reply to: ↑ 71 Changed 8 years ago by AlexanderDreyer

Replying to fbissey:

that helped. Does this make any sense to you

Yeah, it's the streaming of boost-python-based objects via std-streams. Moving hat stuff to a (third) shared lib seems to cure thing. I'm trying to get that into bitbicket now

comment:74 follow-up: Changed 8 years ago by fbissey

You mean site-package/polybori/dynamic/Pypolibori.{so,bundle}, I believe bundle is for frameworks in OS X, sage doesn't build python as a framework I believe (unless the build system default to it).

In any case I don't think so.

One would think that -flat_namespace and -undefined dynamic_lookup are equivalent but may be they aren't.

comment:75 in reply to: ↑ 74 Changed 8 years ago by AlexanderDreyer

So, I pushed my changes to bitbucket. But still ipbori only works when called from lib directory, because the install_name_tool calls are missing up to now. I hope, that it would work, if you add them into your ebuild script. (Maybe I can find a way to add those to scons tomorrow.)

BTW: I had to remove-undefined dynamic_lookup flags. (Otherwise some of the malloc-errors occurred.)

comment:76 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

Changed 8 years ago by AlexanderDreyer

most recent cummulative patch

Changed 8 years ago by AlexanderDreyer

Rebased patch to 4.7.2alpha1

comment:77 Changed 8 years ago by AlexanderDreyer

The spkg together with the rebased patch builds, installs and tests well on an SuSE Enterprise 11 and Mac OS X. It's not clear, whether PolyBoRi? 0.8 is also affected by the incompatibility with newest libm4ri on Gentoo 32Bit, which is mentioned in #11574#27 (I cannot reproduce that issue.)

comment:78 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

The following spkg includes the sources of the newest release candidate http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8-rc.p3.spkg The upstream sources are now compatible with Gentoo-prefix (important for the sage-on-gentoo project) even on Darwin. It also incorporates the fixes of #11574 and #11757, resp., for m4ri compatibility.

Changed 8 years ago by AlexanderDreyer

Additional patch if #11574 is merged

comment:79 Changed 8 years ago by AlexanderDreyer

  • Description modified (diff)

Changed 8 years ago by AlexanderDreyer

Patch for integration PolyBoRi?'s release 0.8.0 (depends on #11574)

comment:80 Changed 8 years ago by AlexanderDreyer

  • Authors set to Alexander Dreyer
  • Cc malb added
  • Dependencies set to #11574
  • Description modified (diff)
  • Owner changed from (none) to AlexanderDreyer
  • Summary changed from Preparing for PolyBoRi's upcoming release 0.8 to Update PolyBoRi to release 0.8.0

Finally, PolyBoRi 0.8 is out. Here's the updated spkg http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.0.p0.spkg

The most recent patch (for devel/sage) is http://trac.sagemath.org/sage_trac/attachment/ticket/11575/polybori-0.8.0.p0.patch

Note that all depends on #11574 now (if someone want's to try out without #11574, please refer to the previous patch).

The tests from devel/sage/sage/rings/ and devel/sage/sage/crypto/ ran successfully (tested with sage-4.7.2alpha1 on SuSE Enterprise 11, Gentoo i486, Mac OSX 10.5 ppc).

comment:81 Changed 8 years ago by AlexanderDreyer

  • Status changed from needs_work to needs_review

comment:82 follow-up: Changed 8 years ago by malb

Mhhh, it fails to build for me on sage.math with 4.7.2.alpha1:

/mnt/usb1/scratch/malb/sage-4.7.2/local/include/csage/ccobject.h: In function ‘T* Construct_p(void*, const P&) [with T = polybori::BooleVariable, P = int]’:
sage/rings/polynomial/pbori.cpp:35286:   instantiated from here
/mnt/usb1/scratch/malb/sage-4.7.2/local/include/csage/ccobject.h:47: error: no matching function for call to ‘polybori::BooleVariable::BooleVariable(const int&)’
/mnt/usb1/scratch/malb/sage-4.7.2/local/include/polybori/BooleVariable.h:100: note: candidates are: polybori::BooleVariable::BooleVariable(const polybori::BoolePolynomial&)
/mnt/usb1/scratch/malb/sage-4.7.2/local/include/polybori/BooleVariable.h:67: note:                 polybori::BooleVariable::BooleVariable(const polybori::BooleVariable&)
/mnt/usb1/scratch/malb/sage-4.7.2/local/include/polybori/BooleVariable.h:63: note:                 polybori::BooleVariable::BooleVariable(const polybori::BoolePolyRing&)
/mnt/usb1/scratch/malb/sage-4.7.2/local/include/polybori/BooleVariable.h:59: note:                 polybori::BooleVariable::BooleVariable(int, const polybori::BoolePolyRing&)
error: command 'gcc' failed with exit status 1

Here's what patches I have applied:

$ hg qap
m4ri_20110601.patch
trac_11574_m4ri_sse2.patch
polybori-0.8.0.p0.patch

comment:83 in reply to: ↑ 82 Changed 8 years ago by AlexanderDreyer

Replying to malb:

Mhhh, it fails to build for me on sage.math with 4.7.2.alpha1:

[...]

Looks like the changes to pbori.pyx were not effective. Did it rebuild pbori.cpp from pbori.pyx?

comment:84 follow-up: Changed 8 years ago by malb

  • Description modified (diff)

Got it, I accidentally applied the HTML description of the patch instead of the patch. Hence, the patch did nothing.

Shall I delete all other attachments btw.?

comment:85 in reply to: ↑ 84 Changed 8 years ago by AlexanderDreyer

Replying to malb:

Got it, I accidentally applied the HTML description of the patch instead of the patch. Hence, the patch did nothing.

I cannot count how often this happened to me :-)

Shall I delete all other attachments btw.?

I prefer to keep them for documentation reasons.

comment:86 Changed 8 years ago by malb

Patch applies cleanly & doctests pass. I skimmed it and it looks good. I'll give it another read but we're quite close to a positive review.

comment:87 Changed 7 years ago by malb

  • Reviewers set to Martin Albrecht
  • Status changed from needs_review to positive_review

Alright, talked to Alexander. Positive review.

comment:88 Changed 7 years ago by malb

  • Status changed from positive_review to needs_work

oops, still needs SSE2 fix applied.

comment:89 Changed 7 years ago by AlexanderDreyer

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

Here's a new spkg with the sse2 fix (backported from PolyBoRi 0.8.1): http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.0.p1.spkg I also had to rebase the patch to for Sage 4.7.2alpha3.

comment:90 Changed 7 years ago by AlexanderDreyer

Here are the essential changes between the patches:

+--- a/sage/rings/polynomial/pbori.pyx
++++ b/sage/rings/polynomial/pbori.pyx  

@@ -1664,16 +1502,24 @@                                                                                              
          if PY_TYPE_CHECK(x, BooleanPolynomial):                                                                    
              return x                                                                                               
 +        elif PY_TYPE_CHECK(x, BooleSet):                                                                           
-+            return (<BooleSet>x)._ring._coerce_(x)                                                                 
++            return (<BooleSet>x)._ring(x)                                                                          
          elif PY_TYPE_CHECK(x, BooleanMonomial):                                                                    
-             return (<BooleanMonomial>x)._ring._coerce_(x)                                                          
+             return (<BooleanMonomial>x)._ring(x)                                                                   
 -#new_BP_from_PBMonom((<BooleanMonomial>x)._ring, (<BooleanMonomial>x)._pbmonom)                                    
+-        else:                                                                                                      
+-            #return get_cring()._coerce_(x)                                                                        
 +        elif PY_TYPE_CHECK(ring, BooleanPolynomialRing):                                                           
-+            return (<BooleanPolynomialRing>ring)._coerce_(x)                                                       
-         else:                                                                                                      
--            return get_cring()._coerce_(x)                                                                         
+             # It is a wrong use of the notion of "coercion"                                                        
+             # to say that the boolean set is "coerced" into                                                        
+             # a boolean polynomial ring: Boolean sets have                                                         
+             # no parent, and thus there is no coercion map                                                         
+             # from that parent to the ring.                                                                        
+             # So, it is just a conversion. [Simon King]                                                            
+-            return get_cring()(x)                                                                                  
 -                                                                                                                   
 -cdef class MonomialFactory:                                                                                        
++            return (<BooleanPolynomialRing>ring)(x)                                                                
++        else:                                                                                                      
 +            raise TypeError, \                                                                                     
 +              "Cannot generate Boolean polynomial from %s , %s%"%(str(type(x)), str(type(ring)))                   
 +                                                                                                                   
@@ -1719,7 +1565,7 @@                                                                                                
 +                   return result.lm()                                                                              
 +                return result                                                                                      
 +            except:                                                                                                
-+                raise #TypeError, "Cannot convert to Boolean Monomial %s"%(str(type(x)))                           
++                raise TypeError, "Cannot convert to Boolean Monomial %s"%(str(type(x)))                            
 +
 +cdef class VariableConstruct:
      """
@@ -3019,7 +2865,7 @@
 +            if PY_TYPE_CHECK(arg, BooleanPolynomial):
 +                return arg
 +            elif PY_TYPE_CHECK(arg, BooleSet):
-+                return (<BooleSet>arg)._ring._coerce_(arg)
++                return (<BooleSet>arg)._ring(arg)
 +            elif PY_TYPE_CHECK(arg, BooleanMonomial):
 +                return (<BooleanMonomial>arg)._ring._coerce_(arg)
 +            elif PY_TYPE_CHECK(ring, BooleanPolynomialRing):

comment:91 Changed 7 years ago by malb

  • Status changed from needs_review to positive_review

doctests work.

comment:92 Changed 7 years ago by PolyBoRi

wow, thanks to all, awesome (as defined in the Oxford english dictionary).

comment:93 Changed 7 years ago by AlexanderDreyer

  • Cc SimonKing added

As discussed with Simon King, I uploaded another patch which does the following:

1505c1505
< +            return (<BooleSet>x)._ring._element_constructor(x)
---
> +            return (<BooleSet>x)._ring(x)
1521c1521
< +            return (<BooleanPolynomialRing>ring)._element_constructor(x)
---
> +            return (<BooleanPolynomialRing>ring)(x)
2868c2868
< +                return (<BooleSet>arg)._ring._element_constructor(arg)
---
> +                return (<BooleSet>arg)._ring(arg)

comment:94 Changed 7 years ago by AlexanderDreyer

  • Status changed from positive_review to needs_work

Note the newest patch uses _element_constructor_ instead of _element_constructor.

comment:95 Changed 7 years ago by AlexanderDreyer

  • Status changed from needs_work to needs_review

comment:96 Changed 7 years ago by AlexanderDreyer

The new splkg/patch build/installs and tests successfully on SLES 11 on amd64.

comment:97 Changed 7 years ago by malb

  • Status changed from needs_review to positive_review

Alright, back to positive review.

comment:98 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-4.7.2 to sage-4.7.3

comment:99 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_work
  • Work issues set to rebase

The Sage library patch should be rebased to sage-4.7.2.alpha3.

comment:100 Changed 7 years ago by AlexanderDreyer

  • Status changed from needs_work to needs_review
  • Work issues rebase deleted

Since the patch applied without fuzz (and passes the tests) I guess the parent checksum in the patch was invalid (indeed it came from some prerelease). The current version of the patch should do its job.

comment:101 Changed 7 years ago by AlexanderDreyer

  • Cc jdemeyer added

Please check, whether this patch applies.

comment:102 Changed 7 years ago by jdemeyer

  • Dependencies changed from #11574 to #11574, #9138
  • Status changed from needs_review to positive_review

Yes, it does apply. Possibly the previous patch also applied. I tried to apply the patch to sage-4.7.2.alpha3 without #9138, this failed. Since the fate of #9138 is not so clear (it could very well be unmerged in sage-4.7.2.alpha4), I will postpone testing this new PolyBoRi until it is clear how #9138 will evolve.

comment:103 follow-up: Changed 7 years ago by leif

Alexander, could you also include (at least) the following:

  • patches/SConstruct

    diff --git a/patches/SConstruct b/patches/SConstruct
    a b  
    545545    env.Append(CPPDEFINES=["BSD"])
    546546
    547547env.Append(LIBPATH=[CuddPath()])
     548if os.environ.has_key("LD_LIBRARY_PATH"):
     549    env.Append(LIBPATH=os.environ["LD_LIBRARY_PATH"].split(os.pathsep))
    548550
    549551cudd_resources = glob(CuddPath('util/*.c'))
    550552

Otherwise the build breaks with "non-standard" compiler setups since PolyBoRi (or SConstruct) invalidates any setting of (e.g.) LDFLAGS, LD_LIBRARY_PATH and LIBRARY_PATH, i.e., removes them from the environment.

The diff is against PolyBoRi 0.7.1.p6.

comment:104 follow-ups: Changed 7 years ago by AlexanderDreyer

Replying to jdemeyer: Please don't tell me you plan to de-merge #9138. It was really hard to get thing rebased to sage-4.7.2.alpha3 because of it. Maybe the PolyBoRi?-related part of #9138 could stay anyway?

comment:105 in reply to: ↑ 103 Changed 7 years ago by AlexanderDreyer

Replying to leif: Hi Leif

env.Append(LIBPATH=os.environLD_LIBRARY_PATH?.split(os.pathsep))

}}}

This could be done in the custom.py file more easily.

Otherwise the build breaks with "non-standard" compiler setups since PolyBoRi (or SConstruct) invalidates any setting of (e.g.) LDFLAGS, LD_LIBRARY_PATH and LIBRARY_PATH, i.e., removes them from the environment.

LD_LIBRARY_PATH is exported to the SCons-enviroment and it works for me. I'll check whether something has hanged in recent SCons versions.

comment:106 Changed 7 years ago by AlexanderDreyer

The PolyBoRi spkg currently uses the following environment variables: CC CXX SAGESOFLAGS PYTHONHOME SAGE_LOCAL

It would be a two liner to support more. Here's what I frequently use: CFLAGS CXXFLAGS CPPFLAGS LDFLAGS Do we need more?

comment:107 in reply to: ↑ 104 Changed 7 years ago by SimonKing

Replying to AlexanderDreyer:

Replying to jdemeyer: Please don't tell me you plan to de-merge #9138. It was really hard to get thing rebased to sage-4.7.2.alpha3 because of it. Maybe the PolyBoRi?-related part of #9138 could stay anyway?

I think the "rescue mission" for #9138 (i.e., ticket #11900) may be successful.

comment:108 in reply to: ↑ 104 ; follow-up: Changed 7 years ago by jdemeyer

Replying to AlexanderDreyer:

Replying to jdemeyer: Please don't tell me you plan to de-merge #9138. It was really hard to get thing rebased to sage-4.7.2.alpha3 because of it. Maybe the PolyBoRi?-related part of #9138 could stay anyway?

First of all, Simon King is working really hard to fix #9138, see #11900. Second, even if #9138 is unmerged in sage-4.7.2.alpha4, it will probably be merged again with #11900, say in sage-4.7.3.alpha0, so there will be no reason to rebase #11575.

comment:109 follow-up: Changed 7 years ago by leif

Replying to AlexanderDreyer:

The PolyBoRi spkg currently uses the following environment variables: CC CXX SAGESOFLAGS PYTHONHOME SAGE_LOCAL

It would be a two liner to support more. Here's what I frequently use: CFLAGS CXXFLAGS CPPFLAGS LDFLAGS Do we need more?

Adding the latter list to the former should be sufficient. Maybe also add LD_RUN_PATH, which is used only if no -rpath parameters are present when linking, and (in general) perhaps ABI and CPP itself.

Other platforms (i.e., operating systems) use different flavours of LD_LIBRARY_PATH etc. though, but I don't care much atm... ;-)

comment:110 in reply to: ↑ 109 ; follow-up: Changed 7 years ago by leif

Replying to leif:

Replying to AlexanderDreyer:

The PolyBoRi spkg currently uses the following environment variables: CC CXX SAGESOFLAGS PYTHONHOME SAGE_LOCAL

It would be a two liner to support more. Here's what I frequently use: CFLAGS CXXFLAGS CPPFLAGS LDFLAGS Do we need more?

Adding the latter list to the former should be sufficient. Maybe also add LD_RUN_PATH, which is used only if no -rpath parameters are present when linking, and (in general) perhaps ABI and CPP itself.

Ooops, and of course LD_LIBRARY_PATH, as mentioned and in the diff.


Other platforms (i.e., operating systems) use different flavours of LD_LIBRARY_PATH etc. though, but I don't care much atm... ;-)

comment:111 in reply to: ↑ 108 Changed 7 years ago by AlexanderDreyer

Replying to jdemeyer:

First of all, Simon King is working really hard to fix #9138, see #11900. Second, even if #9138 is unmerged in sage-4.7.2.alpha4, it will probably be merged again with #11900, say in sage-4.7.3.alpha0, so there will be no reason to rebase #11575.

Very well, Simon is good at these things (one cannot be 100% bug/regression-free in the first). so i have to doubt that he manages the rescue mission. sage-4.7.3.alpha0 is early enough for PoylyBoRi 0.8 anyway.

comment:112 in reply to: ↑ 110 ; follow-up: Changed 7 years ago by AlexanderDreyer

Replying to leif:

Adding the latter list to the former should be sufficient. Maybe also add LD_RUN_PATH, which is used only if no -rpath parameters are present when linking, and (in general) perhaps ABI and CPP itself.

Ooops, and of course LD_LIBRARY_PATH, as mentioned and in the diff.

Ok, but I'll make a new ticket for this as it is not relation to the version update.

comment:113 Changed 7 years ago by AlexanderDreyer

The environment variable issue is #11906.

comment:114 in reply to: ↑ 112 ; follow-up: Changed 7 years ago by leif

Replying to AlexanderDreyer:

Replying to leif:

Adding the latter list to the former should be sufficient. Maybe also add LD_RUN_PATH, which is used only if no -rpath parameters are present when linking, and (in general) perhaps ABI and CPP itself.

Ooops, and of course LD_LIBRARY_PATH, as mentioned and in the diff.

Ok, but I'll make a new ticket for this as it is not relation to the version update.

No problem. I ran into this a while ago, so I have my 0.7.1.p7 spkg.

And I only remembered that when I came across this ticket... ;-)

comment:115 in reply to: ↑ 114 ; follow-up: Changed 7 years ago by AlexanderDreyer

Replying to leif:

No problem. I ran into this a while ago, so I have my 0.7.1.p7 spkg.

And I only remembered that when I came across this ticket... ;-)

But it's a good point anyway. As far as I understand gcc's manual it should be enough to import LIBRARY_PATH into the build environment to avoid your work around.

comment:116 in reply to: ↑ 115 ; follow-ups: Changed 7 years ago by leif

Replying to AlexanderDreyer:

As far as I understand gcc's manual it should be enough to import LIBRARY_PATH into the build environment to avoid your work around.

[In principle belongs to #11906:]

IIRC that didn't help, because the linker doesn't use LIBRARY_PATH but LD_LIBRARY_PATH.

The problem was that it couldn't find [an appropriate] libgcc[_s]; all other paths are compiled into GCC (which used to be in specs). So LDFLAGS would also have helped (if they were used in the link commands).

A work-around is to create symbolic links (in directories where they do not really belong), but since PolyBoRi is the only package that shows this behaviour (or error), I fixed the spkg rather than doing that, which I'd also have to do for a couple of different GCC installations.

comment:117 in reply to: ↑ 116 Changed 7 years ago by leif

Replying to leif:

Replying to AlexanderDreyer:

As far as I understand gcc's manual it should be enough to import LIBRARY_PATH into the build environment to avoid your work around.

IIRC that didn't help, because the linker doesn't use LIBRARY_PATH but LD_LIBRARY_PATH.

Ok, according to the GCC documentation, you are right.

Currently (re)testing that...

comment:118 in reply to: ↑ 116 Changed 7 years ago by AlexanderDreyer

Replying to leif:

IIRC that didn't help, because the linker doesn't use LIBRARY_PATH but LD_LIBRARY_PATH.

LD_LIBRARY_PATH should be used for runtime search, not at link time.

Even though LD_LIBRARY_PATH may be effective at link time as (unwanted?) side effect, I think the missing LDFLAGS caused your problems. (Since LD_LIBRARY_PATH had been imported before.)

comment:119 follow-up: Changed 7 years ago by leif

Replying to leif:

Replying to leif:

Replying to AlexanderDreyer:

As far as I understand gcc's manual it should be enough to import LIBRARY_PATH into the build environment to avoid your work around.

IIRC that didn't help, because the linker doesn't use LIBRARY_PATH but LD_LIBRARY_PATH.

Ok, according to the GCC documentation, you are right.

Currently (re)testing that...

Ok, replacing LD_LIBRARY_PATH with LIBRARY_PATH in my patch above (and setting that to a path along which libgcc* can be found) also works.

But I also noticed other things (when doing ./sage -f ...):

...
Checking for C header file gd.h... yes
Checking for C library gd... no
...
Checking for C library m4ri... no
Checking for C header file gd.h... yes
Checking for C library gd... no
Symlinking to M4RI/m4ri ...
...

(Haven't tracked that further down, but it's strange, provided $SAGE_LOCAL/[lib/] is used there, since both libraries are present.)


Also, spkg-install (I think) shouldn't delete previous versions until the build succeeded.

comment:120 in reply to: ↑ 119 ; follow-up: Changed 7 years ago by AlexanderDreyer

Replying to leif: [...]

Ok, replacing LD_LIBRARY_PATH with LIBRARY_PATH in my patch above (and setting that to a path along which libgcc* can be found) also works.

Try to use the (inlined) patch from #11906. It directly reconstructs the original environment. Even though it will not utilize LDFLAGS yet.

But I also noticed other things (when doing ./sage -f ...):

...
Checking for C header file gd.h... yes
Checking for C library gd... no
...
Checking for C library m4ri... no
Checking for C header file gd.h... yes
Checking for C library gd... no
Symlinking to M4RI/m4ri ...
...

(Haven't tracked that further down, but it's strange, provided $SAGE_LOCAL/[lib/] is used there, since both libraries are present.)

Maybe the patched LIBPATH is corrupted.

Also, spkg-install (I think) shouldn't delete previous versions until the build succeeded.

Then you're possibly using outdated headers.

comment:121 follow-up: Changed 7 years ago by leif

Replying to AlexanderDreyer:

LD_LIBRARY_PATH should be used for runtime search, not at link time.

Also ld uses it, to look up libraries others depend on; libstdc++ has libgcc_s in its DT_NEEDED tags.

If PolyBoRi itself would (directly) need it, its path would have to be in either LDFLAGS or LIBRARY_PATH.

I think the missing LDFLAGS caused your problems.

Alternatively, see above.

(Since LD_LIBRARY_PATH had been imported before.)

Nope, at least not properly; otherwise my patch wouldn't have changed anything.

comment:122 in reply to: ↑ 120 Changed 7 years ago by leif

Replying to AlexanderDreyer:

Replying to leif:

Also, spkg-install (I think) shouldn't delete previous versions until the build succeeded.

Then you're possibly using outdated headers.

A matter of the order of -I...s. PolyBoRi should just prepend the directories of the header files in the spkg.

comment:123 in reply to: ↑ 121 ; follow-up: Changed 7 years ago by AlexanderDreyer

Replying to leif:

I think the missing LDFLAGS caused your problems.

Alternatively, see above.

Are you sure? Does your setup really work without invoking make?

I mean, does plain

g++ -o helloworld helloworld.cc

succeed for a C++ helloworld.cc like the following?

#include <iostream>
int main() {
  std::cout << "Hello World" << std::endl;
  return 0;
}

comment:124 in reply to: ↑ 123 Changed 7 years ago by AlexanderDreyer

Replying to AlexanderDreyer: (Without LIBRARY_PATH of course.)

comment:125 follow-up: Changed 7 years ago by leif

[Trac s*s...]

Replying to AlexanderDreyer:

Replying to leif:

But I also noticed other things (when doing ./sage -f ...):

...
Checking for C header file gd.h... yes
Checking for C library gd... no
...
Checking for C library m4ri... no
Checking for C header file gd.h... yes
Checking for C library gd... no
Symlinking to M4RI/m4ri ...
...

(Haven't tracked that further down, but it's strange, provided $SAGE_LOCAL/[lib/] is used there, since both libraries are present.)

Maybe the patched LIBPATH is corrupted.

I did and do get exactly the same with the vanilla 0.7.1.p6 spkg (which fails to build).

comment:126 in reply to: ↑ 125 ; follow-ups: Changed 7 years ago by AlexanderDreyer

Replying to leif: with the vanilla 0.7.1.p6 spkg (which fails to build). That's weired, what does config.log say?

comment:127 in reply to: ↑ 126 ; follow-up: Changed 7 years ago by AlexanderDreyer

Replying to AlexanderDreyer:

Replying to leif: with the vanilla 0.7.1.p6 spkg (which fails to build). That's weired, what does config.log say?

I just realized that you work around is active after configuring. So trying to link against gd and m4ri fails, becuase the compiler is still broken at that point.

You may add your work around to patches/custom.py (which is executed earlier) as follows:

if os.environ.has_key("LD_LIBRARY_PATH"): 
  LIBPATH += os.environ["LD_LIBRARY_PATH"].split(os.pathsep)

comment:128 Changed 7 years ago by leif

Replying to AlexanderDreyer:

Replying to leif:

I think the missing LDFLAGS caused your problems.

Alternatively, see above.

Are you sure? Does your setup really work without invoking make?

Since (I now remember that) g++ always adds -lgcc_s (also gcc when called with -x c++), even

$ echo "int main(void){return 0;}" > foo.c
$ g++ -x c++ foo.c -o foo

requires libgcc_s.so to be found, e.g. by setting LIBRARY_PATH (unless one uses -static-libgcc, since that's located in other directories).

(I have to set LD_LIBRARY_PATH anyway, such that the runtime linker finds libgmp etc., which are used by gcc and g++. But that's not sufficient to compile an executable like the above as C++. So apparently PolyBoRi, or SConstruct, doesn't really invalidate LD_LIBRARY_PATH -- otherwise the compiler wouldn't have worked at all -- but LIBRARY_PATH, and with my current settings it doesn't matter whether I put the former or the latter into SCons's LIBPATH, since both contain the directory where libgcc_s lives. IIRC I first tried adding things to where the "getenv" dictionary is set up, which didn't work, but I don't recall exactly.)

comment:129 in reply to: ↑ 126 Changed 7 years ago by leif

Replying to AlexanderDreyer:

Replying to leif:

with the vanilla 0.7.1.p6 spkg (which fails to build).

That's weired, what does config.log say?

Well, that's funny:

scons: Configure: Checking for C header file gd.h...
.sconf_temp/conftest_0.c <-
  |
  |#include "gd.h"
  |
  |
gcc -o .sconf_temp/conftest_0.o -c -std=c99 -O3 -Wno-long-long -Wreturn-type -g -fPIC -DNDEBUG -I/tmp/leif/sage-4.7.2.alpha3-gcc-4.4.6/local/include .sconf_temp/conftest_0.c
scons: Configure: yes

scons: Configure: Checking for C library gd...
.sconf_temp/conftest_1.c <-
  |
  |
  |
  |int
  |main() {
  |
  |return 0;
  |}
  |
gcc -o .sconf_temp/conftest_1.o -c -std=c99 -O3 -Wno-long-long -Wreturn-type -g -fPIC -DNDEBUG -I/tmp/leif/sage-4.7.2.alpha3-gcc-4.4.6/local/include .sconf_temp/conftest_1.c
gcc -o .sconf_temp/conftest_1 .sconf_temp/conftest_1.o -L/tmp/leif/sage-4.7.2.alpha3-gcc-4.4.6/local/lib -lgd
/usr/bin/ld: cannot find -lgcc_s
collect2: ld returned 1 exit status
scons: Configure: no

It's not immediately clear what triggers libgcc_s to apparently get pulled in there, since libgd doesn't need it (according to ldd), and it's plain C, compiled with gcc rather than g++.

comment:130 Changed 7 years ago by leif

Ok, it's not -std=c99 (which would be strange anyway), but I now realize that even

$ (unset LIBRARY_PATH; gcc -o foo foo.c)

doesn't work, because

... -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed ...

is always added by that GCC. (At least earlier versions were IMHO smarter w.r.t. this.)

comment:131 in reply to: ↑ 127 ; follow-up: Changed 7 years ago by leif

Replying to AlexanderDreyer:

I just realized that you work around is active after configuring. So trying to link against gd and m4ri fails, becuase the compiler is still broken at that point.

You may add your work around to patches/custom.py (which is executed earlier) as follows:

if os.environ.has_key("LD_LIBRARY_PATH"): 
  LIBPATH += os.environ["LD_LIBRARY_PATH"].split(os.pathsep)

FWIW, both adding

if os.environ.has_key("LIBRARY_PATH"): 
    LIBPATH += os.environ["LIBRARY_PATH"].split(os.pathsep)

(LIBRARY_PATH, not LD_LIBRARY_PATH) in patches/custom.py as well as changing patches/SConstruct to

# Get paths an related things from current environment
# todo: Are these settings sane in any case?
getenv = dict()
# for key in ['PATH', 'HOME', 'LD_LIBRARY_PATH'] :
for key in ['PATH', 'HOME', 'LD_LIBRARY_PATH', 'LIBRARY_PATH'] :
    try:
        getenv[key] = os.environ[key]
    except KeyError:
        pass

independently work for me, i.e., in both cases the build succeeds, and the GD and M4RI libraries are found by "Configure".


Btw., it would be safer (and save a little space) to remove the M4RI source tree from the PolyBoRi spkg.

comment:132 in reply to: ↑ 131 ; follow-up: Changed 7 years ago by AlexanderDreyer

Replying to leif:

FWIW, both adding

if os.environ.has_key("LIBRARY_PATH"): 
    LIBPATH += os.environ["LIBRARY_PATH"].split(os.pathsep)

(LIBRARY_PATH, not LD_LIBRARY_PATH) in patches/custom.py as well as changing patches/SConstruct to

# Get paths an related things from current environment
# todo: Are these settings sane in any case?
getenv = dict()
# for key in ['PATH', 'HOME', 'LD_LIBRARY_PATH'] :
for key in ['PATH', 'HOME', 'LD_LIBRARY_PATH', 'LIBRARY_PATH'] :
    try:
        getenv[key] = os.environ[key]
    except KeyError:
        pass

independently work for me, i.e., in both cases the build succeeds, and the GD and M4RI libraries are found by "Configure".

Excellent! So this is in fact #11906, we can continue there.

Btw., it would be safer (and save a little space) to remove the M4RI source tree from the PolyBoRi spkg.

Good idea!

comment:133 in reply to: ↑ 132 Changed 7 years ago by leif

Replying to AlexanderDreyer:

Replying to leif:

Btw., it would be safer (and save a little space) to remove the M4RI source tree from the PolyBoRi spkg.

Good idea!

Cf. #9472... ;-)

comment:134 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-4.7.3 to sage-pending

comment:135 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_work
  • Work issues set to cython-0.15.1

The file sage/rings/polynomial/pbori.pyx fails to compile with Cython-0.15 (#11761) because Cython is more strict about undeclared variables. Since Cython-0.15 is likely to be merged before or at the same time with this ticket, this should be fixed.

Error compiling Cython file:
------------------------------------------------------------
...
            ...
            TypeError: argument must be a BooleanPolynomial.
        """
        from polybori import red_tail
        if not I:
            return g
                   ^
------------------------------------------------------------

sage/rings/polynomial/pbori.pyx:4538:20: local variable 'g' referenced before assignment

comment:136 Changed 7 years ago by AlexanderDreyer

Indeed, the line in question was buggy. So I corrected it and added a suitable doctest:

  • sage/rings/polynomial/pbori.pyx

    diff --git a/sage/rings/polynomial/pbori.pyx b/sage/rings/polynomial/pbori.pyx
    a b  
    45164516            0
    45174517            sage: p.reduce(I)
    45184518            x1*x2*x3 + x2
    4519 
     4519            sage: p.reduce([])
     4520            x0*x1*x2 + x0*x1*x3 + x0*x2*x3 + x2
     4521
    45204522        .. note::
    45214523
    45224524           If this function is called repeatedly with the same I then
     
    45354537        """
    45364538        from polybori import red_tail
    45374539        if not I:
    4538             return g
     4540            return self
    45394541        if PY_TYPE_CHECK(I, BooleanPolynomialIdeal):
    45404542            I = I.gens()
    45414543        first = I[0]

Changed 7 years ago by AlexanderDreyer

Patch rebased to Sage 4.7.2 alpha3 (+ deps), fixed cygwin 0.15 issue

comment:137 Changed 7 years ago by AlexanderDreyer

  • Status changed from needs_work to needs_review

I uploaded a revised patch (with the same name).

comment:138 follow-up: Changed 7 years ago by malb

  • Status changed from needs_review to needs_work
  • Work issues cython-0.15.1 deleted

Something seems to have gone wrong with the rebased patch:

I

This gave me a few hunks and one conflict in PolynomialConstruct which I fixed. While it now compiles I get these failures:

File "/opt/sage-4.7.2-polybori-0.8/devel/sage/sage/rings/polynomial/pbori.pyx", line 3859:
    sage: loads(dumps(a)) == a
Exception raised:
    Traceback (most recent call last):
    ...
    AttributeError: ('sage.rings.polynomial.pbori.BooleanPolynomialRing' object has no attribute '_element_constructor_', <built-in function unpickle_BooleanPolynomial0>, (Boolean PolynomialRing in a, b, ([[2], [{{a}}]], Boolean PolynomialRing in a, b)))
**********************************************************************
File "/opt/sage-4.7.2-polybori-0.8/devel/sage/sage/rings/polynomial/pbori.pyx", line 4487:
    sage: PolynomialConstruct()(1)
Exception raised:
    Traceback (most recent call last):
    ...
    ValueError: incomplete format
**********************************************************************
File "/opt/sage-4.7.2-polybori-0.8/devel/sage/sage/rings/polynomial/pbori.pyx", line 7445:
    sage: loads(dumps(a+b)) == a+b # indirect doctest
Exception raised:
    Traceback (most recent call last):
    ...
    AttributeError: ('sage.rings.polynomial.pbori.BooleanPolynomialRing' object has no attribute '_element_constructor_', <built-in function unpickle_BooleanPolynomial0>, (Boolean PolynomialRing in a, b, c, d, ([[2], [{{a}, {b}}, {{b}}]], Boolean PolynomialRing in a, b, c, d)))
**********************************************************************
File "/opt/sage-4.7.2-polybori-0.8/devel/sage/sage/rings/polynomial/pbori.pyx", line 7459:
    sage: loads(dumps(a+b)) == a+b # indirect doctest
Exception raised:
    Traceback (most recent call last):
    ...
    AttributeError: ('sage.rings.polynomial.pbori.BooleanPolynomialRing' object has no attribute '_element_constructor_', <built-in function unpickle_BooleanPolynomial0>, (Boolean PolynomialRing in a, b, c, d, ([[2], [{{a}, {b}}, {{b}}]], Boolean PolynomialRing in a, b, c, d)))
**********************************************************************

So something went wrong and Alexander's element_constructor fixes didn't make it into this patch?

comment:139 in reply to: ↑ 138 Changed 7 years ago by jdemeyer

Replying to malb:

Something seems to have gone wrong with the rebased patch:

I

Did you apply the dependency #9138?

comment:140 Changed 7 years ago by malb

Ha, no! Sorry for the noise then.

comment:141 Changed 7 years ago by malb

  • Status changed from needs_work to needs_review

Works as advertised, but I didn't try it with the new Cython yet.

comment:142 Changed 7 years ago by malb

  • Status changed from needs_review to positive_review

Works with Sage 4.8.alpha1.

comment:143 Changed 7 years ago by jdemeyer

Could you re-export the patch with a username added? Maybe you need to create a $HOME/.hgrc file? See instructions at http://sagemath.org/doc/developer/walk_through.html#submitting-a-change

comment:144 Changed 7 years ago by AlexanderDreyer

Thank for pointing this out. I wasn't aware, that hg's mq doesn't write the username into the patch header. Now, I regenerated the patch using hg export tip.

comment:145 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:146 Changed 7 years ago by AlexanderDreyer

@jdemeyer Since #9138 (and its dependencies) is now about to enter Sage 5.0, could we change the milestone to sage-5.0 here? What do you thing?

comment:147 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.0

Changed 7 years ago by jdemeyer

Rebased to latest #9138

comment:148 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.0.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.