Opened 9 years ago
Closed 8 years ago
#13767 closed enhancement (fixed)
upgrade boost to version 1.52.0
Reported by: | tkluck | Owned by: | tbd |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.8 |
Component: | packages: standard | Keywords: | |
Cc: | jpflori | 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 9 years ago by
comment:2 Changed 9 years ago by
- Status changed from new to needs_review
comment:3 Changed 8 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 8 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 8 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 8 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 8 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 8 years ago by
- Description modified (diff)
- Reviewers set to Volker Braun
comment:9 Changed 8 years ago by
- Status changed from needs_review to positive_review
Looks good to me
comment:10 follow-up: ↓ 12 Changed 8 years ago by
- Cc jpflori 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 8 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 in reply to: ↑ 10 ; follow-up: ↓ 14 Changed 8 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 in reply to: ↑ 11 Changed 8 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 in reply to: ↑ 12 Changed 8 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 8 years ago by
- Status changed from positive_review to 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 in reply to: ↑ 15 Changed 8 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 8 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:18 Changed 8 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 8 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:20 Changed 8 years ago by
Edited: it should read len(result) == n
.
comment:21 follow-up: ↓ 22 Changed 8 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 in reply to: ↑ 21 ; follow-up: ↓ 23 Changed 8 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 in reply to: ↑ 22 ; follow-up: ↓ 26 Changed 8 years ago by
- Reviewers changed from Volker Braun to Volker Braun, fbissey, Alexander Dreyer
- Status changed from needs_review to 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 8 years ago by
- Status changed from positive_review to 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 8 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 in reply to: ↑ 23 ; follow-up: ↓ 27 Changed 8 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 in reply to: ↑ 26 ; follow-up: ↓ 28 Changed 8 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 in reply to: ↑ 27 Changed 8 years ago by
(You wrote it correctly, I just misread)
Changed 8 years ago by
Changed 8 years ago by
comment:29 Changed 8 years ago by
- Status changed from needs_work to 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 8 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 8 years ago by
- Reviewers changed from Volker Braun, fbissey, Alexander Dreyer to 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 in reply to: ↑ 31 Changed 8 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 in reply to: ↑ 24 Changed 8 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 8 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 8 years ago by
- Status changed from needs_review to positive_review
All administrative issues are fixed.
comment:36 in reply to: ↑ 34 Changed 8 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 8 years ago by
- Merged in set to sage-5.8.beta3
- Resolution set to fixed
- Status changed from positive_review to 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