Opened 10 years ago

Closed 10 years ago

#13767 closed enhancement (fixed)

upgrade boost to version 1.52.0

Reported by: Timo Kluck Owned by: tbd
Priority: minor Milestone: sage-5.8
Component: packages: standard Keywords:
Cc: Jean-Pierre Flori Merged in: sage-5.8.beta3
Authors: Timo Kluck Reviewers: Volker Braun, François Bissey, Alexander Dreyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Timo Kluck)

There was a discussion on sage-devel a while ago about including the Boost headers [1]. As noted there, the latest version of the optional spkg polymake doesn't compile with the current boost package. The current boost package is called boost-cropped because it contains only a subset of the boost headers, because of package size.

This new package is contains all headers, which is still only a subset of the upstream tarball. The total packaged size is ~ 5M, which feels reasonable to me.

I've checked that the current version polybori-0.8.2 and the current upstream version polymake-2.12-rc3 both compile under Sage with these headers. I'm also submitting an spkg for polymake in another ticket.

[1] https://groups.google.com/d/topic/sage-devel/fcxNrQqVSz0/discussion

Attachments (2)

0001-Update-doctests-for-polybori-that-rely-on-randomness.patch (1.2 KB) - added by Timo Kluck 10 years ago.
trac-13767-sage_root-upgrade-to-boost-1.52.0.patch (600 bytes) - added by Timo Kluck 10 years ago.

Download all attachments as: .zip

Change History (39)

comment:1 Changed 10 years ago by Timo Kluck

Because of the file size limit on trac attachment, I've put it here:

http://www.infty.nl/misc/boost-cropped-1.52.0.spkg

comment:2 Changed 10 years ago by Timo Kluck

Status: newneeds_review

comment:3 Changed 10 years ago by Volker Braun

The deleted patch is still in the repo:

[vbraun@laptop boost-cropped-1.52.0]$ hg st
! patches/boost-1.34.1-gcc-4.4.0-fixes.patch

Also, I take it the following in SPKG.txt is no longer true?

Merged against policy a patch that makes boost build with gcc 4.4.0. Will be obsoleted by an updated boost shortly.

comment:4 Changed 10 years ago by Timo Kluck

Thanks for reviewing. I've removed that line from SKPG.txt, and committed my changes into the hg repo. I also renamed the package to boost_cropped (underscore instead of dash) to conform to the name-version scheme.

The current version is here:

http://www.infty.nl/misc/boost_cropped-1.52.0.spkg

comment:5 Changed 10 years ago by Timo Kluck

I just uploaded a slightly newer version (md5sum d897d8e152623fab9f9c866ce184b501) that fixes a bash escaping error in spkg-install that I happened to notice.

comment:6 Changed 10 years ago by Volker Braun

+1 for underscore but don't we have to patch those as well?

[vbraun@laptop Sage]$ egrep boost-cropp sage/spkg/*
sage/spkg/install:BOOST_CROPPED=`newest_version boost-cropped`
sage/spkg/Makefile:BOOST_CROPPED=boost-cropped-1.34.1

comment:7 Changed 10 years ago by Timo Kluck

I attached a patch doing just that. I don't know how to instruct the patchbot to apply this to sage_root, and it wouldn't work anyway since it needs the new spkg. Let me know if I can do anything more, though.

Note that the file spkg/Makefile is automatically generated, so I didn't patch it.

Last edited 10 years ago by Timo Kluck (previous) (diff)

comment:8 Changed 10 years ago by Volker Braun

Description: modified (diff)
Reviewers: Volker Braun

comment:9 Changed 10 years ago by Volker Braun

Status: needs_reviewpositive_review

Looks good to me

comment:10 Changed 10 years ago by Karl-Dieter Crisman

Cc: Jean-Pierre Flori added

Dumb question: Will the change in package name (though salutary) affect upgrading? (I.e., would we need a build patch for that?)

CC:ing JP in case this might help any Cygwin woes with not having certain function (along the lines of what cephes was supposed to accomplish, but wasn't accomplishing since it wasn't installed...), though looks like he's gotten most of them ironed out :)

comment:11 Changed 10 years ago by Jeroen Demeyer

In SPKG.txt, replace

=== boost-cropped-1.52.0 (Timo Kluck, November 28, 2012) ===

by

=== boost_cropped-1.52.0 (Timo Kluck, November 28, 2012) ===

comment:12 in reply to:  10 ; Changed 10 years ago by Jeroen Demeyer

Description: modified (diff)

Replying to kcrisman:

Dumb question: Will the change in package name (though salutary) affect upgrading?

Yes.

(I.e., would we need a build patch for that?)

Yes, but there is such a patch.

comment:13 in reply to:  11 Changed 10 years ago by Timo Kluck

Replying to jdemeyer:

In SPKG.txt, replace

=== boost-cropped-1.52.0 (Timo Kluck, November 28, 2012) ===

by

=== boost_cropped-1.52.0 (Timo Kluck, November 28, 2012) ===

Done and set the date to today. I uploaded a new version (md5sum 2e12e082e9db4e908d0796246d0af351).

comment:14 in reply to:  12 Changed 10 years ago by Karl-Dieter Crisman

(I.e., would we need a build patch for that?)

Yes, but there is such a patch.

Ok, I see that now. I will trust you that it will perform appropriately :)

comment:15 Changed 10 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

Could this have caused

sage -t  --long -force_lib devel/sage/sage/rings/polynomial/pbori.pyx
**********************************************************************
File "/release/merger/sage-5.8.beta2/devel/sage-main/sage/rings/polynomial/pbori.pyx", line 7868:
    sage: random_set((a*b*c*d).lm(),2)
Expected:
    {{b,c}, {b}}
Got:
    {{b}, {c}}
**********************************************************************
[...more like this...]

comment:16 in reply to:  15 Changed 10 years ago by Timo Kluck

Replying to jdemeyer:

Could this have caused

sage -t  --long -force_lib devel/sage/sage/rings/polynomial/pbori.pyx
**********************************************************************
File "/release/merger/sage-5.8.beta2/devel/sage-main/sage/rings/polynomial/pbori.pyx", line 7868:
    sage: random_set((a*b*c*d).lm(),2)
Expected:
    {{b,c}, {b}}
Got:
    {{b}, {c}}
**********************************************************************
[...more like this...]

Yes, I'm pretty sure it has: I can reproduce this issue on my stock 5.5 installation with upgraded boost and recompiled polybori.

The reason is possibly that polybori probably relies on Boost.Random for its randomness. Since we're doing a rather large update 1.34 - > 1.52, it's quite possible that the implementation has changed sufficiently to obtain different random values even with the same seed.

I would be inclined to just update the doctests, since these are inherently random results. But I would prefer to first hear the opinion of someone who actually uses polybori.

comment:17 Changed 10 years ago by Timo Kluck

Description: modified (diff)
Status: needs_workneeds_review

comment:18 Changed 10 years ago by François Bissey

I approve the patch for the polybori tests. I have seen exactly this output in sage-on-gentoo for the last few months. I have it documented at least as far back as the time of sage-5.4.

comment:19 Changed 10 years ago by Alexander Dreyer

These random ;-) changes in PolyBoRi's random functions won't cause any problems.

But maybe we should just ignore the doctest results and add additional tests for the following assumed properties result = random_set(monomial, n) should obey len(result) == n and result.diff(monomial.divisors()).empty() .

Last edited 10 years ago by Alexander Dreyer (previous) (diff)

comment:20 Changed 10 years ago by Alexander Dreyer

Edited: it should read len(result) == n.

comment:21 Changed 10 years ago by Alexander Dreyer

Anyway, if we need a quick fix: I can also give a positive review. The more generic test would then be a new ticket.

comment:22 in reply to:  21 ; Changed 10 years ago by Timo Kluck

Replying to AlexanderDreyer:

Anyway, if we need a quick fix: I can also give a positive review. The more generic test would then be a new ticket.

That would be great; the polymake upgrade also depends on this ticket, so it would be great to just get it in.

comment:23 in reply to:  22 ; Changed 10 years ago by Alexander Dreyer

Reviewers: Volker BraunVolker Braun, fbissey, Alexander Dreyer
Status: needs_reviewpositive_review

Replying to tkluck:

That would be great; the polymake upgrade also depends on this ticket, so it would be great to just get it in.

Allright then. Please set up the other ticket when there is time and CC me there.

Just out of curiosity: how is the polymake ticket related to PolyBoRi?

comment:24 Changed 10 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

Could you make SPKG.txt world-readable inside the spkg:

1435597707    4 -rw-------   1 jdemeyer jdemeyer     1406 Feb 26 22:22 ./SPKG.txt

comment:25 Changed 10 years ago by Jeroen Demeyer

Also: the two patches are not proper Mercurial changesets (they lack a header). You should generate patches using hg export tip (see http://sagemath.org/doc/developer/walk_through.html#creating-a-change or http://sagemath.org/doc/developer/walk_through.html#creating-your-own-patch-with-queues)

comment:26 in reply to:  23 ; Changed 10 years ago by Timo Kluck

Replying to AlexanderDreyer:

That would be great; the polymake upgrade also depends on this ticket, so it would be great to just get it in.

Just out of curiosity: how is the polymake ticket related to PolyBoRi?

This is not a polybori ticket, but a boost ticket :-)

comment:27 in reply to:  26 ; Changed 10 years ago by Alexander Dreyer

Replying to tkluck:

This is not a polybori ticket, but a boost ticket :-)

Sorry, I misunderstood this. (Ticket vs. patch.)

comment:28 in reply to:  27 Changed 10 years ago by Alexander Dreyer

(You wrote it correctly, I just misread)

Last edited 10 years ago by Alexander Dreyer (previous) (diff)

comment:29 Changed 10 years ago by Timo Kluck

Status: needs_workneeds_review

The patches are now hg changesets. Sorry about that, I've been doing development on an integrated git tree recently, and I still had to setup the conversion git -> hg.

comment:30 Changed 10 years ago by Timo Kluck

I created #14205 for possibly making those doctests non-random, and put Alexander in the CC as requested.

comment:31 Changed 10 years ago by Karl-Dieter Crisman

Reviewers: Volker Braun, fbissey, Alexander DreyerVolker Braun, François Bissey, Alexander Dreyer

Could this be dependent on platform (the pbori tests, I mean)? I get all the original ones after installing this spkg and patches, doing ./sage -b and testing (hence failures).

----------------------------------------------------------------------
| Sage Version 5.8.beta1, Release Date: 2013-02-24                   |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
sage: from polybori import random_set, set_random_seed
sage: B.<a,b,c,d,e> = BooleanPolynomialRing()
sage: (a*b*c*d).lm()
a*b*c*d
sage: set_random_seed(1337)
sage: random_set((a*b*c*d).lm(),2)
{{b,c}, {b}}

Is there something else I should have done to make sure this change works?

Version 0, edited 10 years ago by Karl-Dieter Crisman (next)

comment:32 in reply to:  31 Changed 10 years ago by Timo Kluck

Replying to kcrisman:

Could this be dependent on platform (the pbori tests, I mean)? I get all the original ones after installing this spkg and patches, doing ./sage -b and testing (hence failures). Is there something else I should have done to make sure this change works? (I'm on an Intel Mac OSX 10.7.)

Did you recompile the Polybori package?

sage -f polybori

comment:33 in reply to:  24 Changed 10 years ago by Timo Kluck

Replying to jdemeyer:

Could you make SPKG.txt world-readable inside the spkg:

1435597707    4 -rw-------   1 jdemeyer jdemeyer     1406 Feb 26 22:22 ./SPKG.txt

This has been done, too. New package version with md5sum 2852b9e02ede0ab2785930d0f9268c6f.

comment:34 Changed 10 years ago by Karl-Dieter Crisman

Did you recompile the Polybori package?

No, because it didn't say to. Maybe there should be something that retriggers this on update? Anyway, works fine now, sorry for the noise (on the way to #13768...)

comment:35 Changed 10 years ago by Jeroen Demeyer

Status: needs_reviewpositive_review

All administrative issues are fixed.

comment:36 in reply to:  34 Changed 10 years ago by Timo Kluck

Replying to kcrisman:

Did you recompile the Polybori package?

No, because it didn't say to. Maybe there should be something that retriggers this on update?

You're right. The makefile spkg/Makefile knows which packages depend on which others. Your comment reminded me to check whether it knows that polybori depends on boost_cropped: it does. I'm pretty sure that means that the upgrade path will work.

Thanks for testing!

comment:37 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.8.beta3
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.