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: |
Description (last modified by )
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
- Apply trac-13767-sage_root-upgrade-to-boost-1.52.0.patch to the root repo.
- Replace boost-cropped spkg with http://www.infty.nl/misc/boost_cropped-1.52.0.spkg
- Apply 0001-Update-doctests-for-polybori-that-rely-on-randomness.patch to the sage-main repo.
Attachments (2)
Change History (39)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Status: | new → needs_review |
---|
comment:3 Changed 10 years ago by
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
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:
comment:5 Changed 10 years ago by
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
+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
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.
comment:8 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Reviewers: | → Volker Braun |
comment:10 follow-up: 12 Changed 10 years ago by
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 follow-up: 13 Changed 10 years ago by
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 follow-up: 14 Changed 10 years ago by
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 Changed 10 years ago by
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 Changed 10 years ago by
(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 follow-up: 16 Changed 10 years ago by
Status: | positive_review → needs_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 Changed 10 years ago by
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
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
comment:18 Changed 10 years ago by
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
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()
.
comment:21 follow-up: 22 Changed 10 years ago by
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 follow-up: 23 Changed 10 years ago by
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 follow-up: 26 Changed 10 years ago by
Reviewers: | Volker Braun → Volker Braun, fbissey, Alexander Dreyer |
---|---|
Status: | needs_review → positive_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 follow-up: 33 Changed 10 years ago by
Status: | positive_review → needs_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
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 follow-up: 27 Changed 10 years ago by
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 follow-up: 28 Changed 10 years ago by
Replying to tkluck:
This is not a polybori ticket, but a boost ticket :-)
Sorry, I misunderstood this. (Ticket vs. patch.)
comment:28 Changed 10 years ago by
(You wrote I correctly, I just misread)
Changed 10 years ago by
Attachment: | 0001-Update-doctests-for-polybori-that-rely-on-randomness.patch added |
---|
Changed 10 years ago by
Attachment: | trac-13767-sage_root-upgrade-to-boost-1.52.0.patch added |
---|
comment:29 Changed 10 years ago by
Status: | needs_work → needs_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
I created #14205 for possibly making those doctests non-random, and put Alexander in the CC as requested.
comment:31 follow-up: 32 Changed 10 years ago by
Reviewers: | Volker Braun, fbissey, Alexander Dreyer → Volker 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? (I'm on an Intel Mac OSX 10.7.)
comment:32 Changed 10 years ago by
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 Changed 10 years ago by
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 follow-up: 36 Changed 10 years ago by
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
Status: | needs_review → positive_review |
---|
All administrative issues are fixed.
comment:36 Changed 10 years ago by
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
Merged in: | → sage-5.8.beta3 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
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