Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade MPFR to 3.1.0 #11666

Closed
nexttime mannequin opened this issue Aug 8, 2011 · 118 comments
Closed

Upgrade MPFR to 3.1.0 #11666

nexttime mannequin opened this issue Aug 8, 2011 · 118 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 8, 2011

Our current MPFR spkg is fairly old (based on MPFR 2.4.2), and upgrading it to the latest [stable] upstream release is now on the high-priority wishlist.

Use http://perso.telecom-paristech.fr/~flori/sage/mpfr-3.1.0.p0.spkg

Apply:

Depends on #12171
Depends on #12548

Upstream: Fixed upstream, but not in a stable release.

CC: @jpflori @zimmermann6

Component: packages: standard

Keywords: sd36 sd32 MPFR spkg wishlist sd35.5

Author: Mike Hansen, Jean-Pierre Flori, Volker Braun

Reviewer: Paul Zimmermann, Jean-Pierre Flori, Volker Braun, Jeroen Demeyer

Merged: sage-5.0.beta7

Issue created by migration from https://trac.sagemath.org/ticket/11666

@nexttime nexttime mannequin added this to the sage-5.0 milestone Aug 8, 2011
@williamstein
Copy link
Contributor

Changed keywords from MPFR spkg wishlist to sd32 MPFR spkg wishlist

@nexttime

This comment has been minimized.

@nexttime nexttime mannequin self-assigned this Sep 18, 2011
@nexttime nexttime mannequin changed the title Upgrade MPFR to 3.0.1 + upstream patches Upgrade MPFR to 3.0.1 + upstream patches or later (3.1.0) Sep 18, 2011
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 22, 2011

comment:4

The second release candidate is out:

[...] The main changes since this first release candidate are:

  • Fixed --enable-gmp-internals.
  • Handle the special cases in mpfr_cmp_q() and mpfr_cmp_f() (fixing the problem with the erange flag in particular).
  • Added mpfr_buildopt_gmpinternals_p() function.
  • MPFR manual update and minor corrections.

Here's a second release candidate. As there should not be new platform-specific problems, the final release is delayed by a few days only.

[...]

http://www.mpfr.org/mpfr-3.1.0/mpfr-3.1.0-rc2.tar.bz2

[...]

The MD5's:

[...]

6ba48c87687959d5e68cd695686257f4 mpfr-3.1.0-rc2.tar.bz2

[...]

[...]

Final release now scheduled for October 3rd.

@nexttime

This comment has been minimized.

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Oct 3, 2011

comment:5

The final MPFR 3.1.0 has been released today.

See the description for links.

@mwhansen
Copy link
Contributor

Dependencies: 12171

@mwhansen
Copy link
Contributor

Author: Mike Hansen

@mwhansen mwhansen changed the title Upgrade MPFR to 3.0.1 + upstream patches or later (3.1.0) Upgrade MPFR to 3.1.0 Dec 17, 2011
@mwhansen
Copy link
Contributor

comment:7

I have an spkg at http://sage.math.washington.edu/mpfr-3.1.0.spkg. This is needed for flint2.

@kiwifb
Copy link
Member

kiwifb commented Dec 17, 2011

comment:8

Shouldn't the dependency be the other way around? That is mpfi needs mpfr rather than mpfr needs mpfi? Have you tested this on a 32bit OS?

@mwhansen
Copy link
Contributor

comment:9

This version of MPFI should work with both versions of MPFR. However, the old version of MPFI will not work with the new version of MPFI.

I haven't tested it on a 32bit OS. This was primarily just so we could build flint2 in Sage.

@zimmermann6
Copy link

comment:11

I wonder why comment [comment:8] speaks about MPFI. Also, upstream (MPFR) we have seen several
problems with the --enable-thread-safe option which is now enabled by default. Unless needed
for Sage, my advice would be to configure MPFR with --disable-thread-safe.

Paul

@zimmermann6
Copy link

comment:12

the url from comment [comment:7] does not work.

Paul

@kiwifb
Copy link
Member

kiwifb commented Dec 18, 2011

comment:13

Replying to @zimmermann6:

I wonder why comment [comment:8] speaks about MPFI. Also, upstream (MPFR) we have seen several
problems with the --enable-thread-safe option which is now enabled by default. Unless needed
for Sage, my advice would be to configure MPFR with --disable-thread-safe.

I spoke about mpfi because this ticket is marked as depending on #12171 which is upgrading mpfi. My understanding of the dependency field is that it means that you need #12171 for this ticket. This is silly and should be the other way around.

I think comment 9 has not been written with a clear mind.

I asked about test on 32bit machine because of this [ https://github.com/cschwan/sage-on-gentoo/issues/13].

@mwhansen
Copy link
Contributor

comment:14

The spkg is actually at http://sage.math.washington.edu/home/mhansen/mpfr-3.1.0.spkg

Let me clarify the MPFI issue since I must have been too tired when I tried before. If we were to just install this version of MPFR, the current MPFI (1.3.4) will fail to build since it uses something which has been removed/moved in the new version of MPFR. If the new version of MPFI works with both the old and new versions of MPFR. So, if we want to actually include this package in Sage, we have to have the new version of MPFI in first. This is the reason I listed the MPFI ticket as a dependency of this one.

@kiwifb
Copy link
Member

kiwifb commented Dec 19, 2011

comment:15

You are making sense now. I indeed remember making a patch for mpfi-1.4 to build against mpfr-3.x in gentoo, it would apply to 1.3.4 as well I think. Anyway there are a number of things to fix as far as I can see because of the inclusion of these two.

@jpflori
Copy link

jpflori commented Jan 8, 2012

comment:16

Some thoughts about the spkg-install script:

  • It sets some CFLAGS and others itself, is it needed? does not mpfr look for the gmp header and pumps from there if possible? and should we take advantage of this (as in done for mpir where spkg-install look at what mpir chooses if CFLAGS et al. are empty)?
  • Is the patch for Solaris stuff still needed? (see MPFR test failures on Solaris 10 update 4 on host 't2' #6453) -> at least the code in mpn_exp.c did not change in mpfr 3.1.0, so I guess it has to be included to support outdated Solaris systems where memset is buggy?
  • It always runs the test suite, whether SAGE_CHECK is set (then it is ran twice...) or not (then just once), is there still a good reason for doing so? It seems that such a behavior was also present in the MPIR spkg but was removed (see MPIR: Don't check SAGE_CHECK in spkg-install #9522 and !Upgrade Sage's MPIR spkg to version 2.1.3 #8664)
  • make should be replaced by $MAKE in spkg-check

@zimmermann6
Copy link

comment:17

It sets some CFLAGS and others itself, is it needed?

as a developer of MPFR I strongly recommend to let MPFR choose the same CC and CFLAGS as GMP
(they are read in gmp.h). Since we do this for MPFR, this greatly improved the portability of
MPFR on different platforms with different compilers, in particuler it solves the ABI=32 vs ABI=64
issues.

Is the patch for Solaris stuff still needed?

I guess yes, since this was a bug in the Solaris kernel.

Paul

@zimmermann6
Copy link

comment:18

I've tested this spkg on top of #12171 on my 64-bit laptop (Core 2 Duo): all doctests pass.

Paul

@kiwifb
Copy link
Member

kiwifb commented Jan 9, 2012

comment:19

Replying to @zimmermann6:

I've tested this spkg on top of #12171 on my 64-bit laptop (Core 2 Duo): all doctests pass.

Yes 64bit is fine as far as I can tell. It's 32bit that's the trouble - both linux and OS X [10.5.8], don't know about other like solaris or cygwin.

@jpflori
Copy link

jpflori commented Feb 26, 2012

comment:77

No, I did not. I just started on top of Mikes package which came before your comment and I don't remmeber that MPFR includes that flag by default. Ill send a new package tomorrow if you think that's needed. Too late for today.

@vbraun
Copy link
Member

vbraun commented Feb 26, 2012

comment:78

I'm pretty sure we don't use multithreading with MPFR so we can just pass --disable-thread-safe to configure. I'll be happy to review it tomorrow ;-)

@vbraun
Copy link
Member

vbraun commented Feb 26, 2012

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.

@zimmermann6
Copy link

comment:79

Jean-Pierre, the -disable-thread-safe is not needed, but since Sage does not use
multithreading with MPFR, and given the poor support of multithreading by some compilers and operating systems, I recommend disabling thread local storage.

Paul

@jpflori
Copy link

jpflori commented Feb 27, 2012

comment:80

SPKG updated at the same address.

@jpflori
Copy link

jpflori commented Mar 2, 2012

comment:81

Is there anything preventing this from getting positive review ?

The package address did not change, so I've not updated the ticket description.

@zimmermann6
Copy link

comment:82

Here is a review of trac_11666_spkg.patch:
there are small typos in SPKG.txt, which were already there before:
coment should be comment,
occured should be occurred,
reccommened should be recommended.

Apart from that, the more important would be to test the modified spkg.

If this was already tested by the buildbot, then I will give a positive review.

Paul Zimmermann

@jpflori
Copy link

jpflori commented Mar 2, 2012

comment:83

From http://wiki.sagemath.org/buildbot it seems that the patchbot does not feel concerned by automatically testing spkg and I have no idea how to use Sage facilities to perform automatically such tests. I even think I have no account on the needed machines, so I cannot take care of launching automated tests.

Anyway, I'll take the typos into account tomorrow and update the spkg.

@jpflori
Copy link

jpflori commented Mar 2, 2012

Work Issues: typos

@jpflori
Copy link

jpflori commented Mar 2, 2012

comment:84

In fact I had some time right now and corrected the typos.

The package is at the same address as before.

@jpflori
Copy link

jpflori commented Mar 2, 2012

Attachment: trac_11666_spkg.patch.gz

spkg diff, for review only

@jdemeyer
Copy link

jdemeyer commented Mar 2, 2012

comment:85

Replying to @jpflori:

From http://wiki.sagemath.org/buildbot it seems that the patchbot does not feel concerned by automatically testing spkg and I have no idea how to use Sage facilities to perform automatically such tests. I even think I have no account on the needed machines, so I cannot take care of launching automated tests.

Yes, there is currently no way to automatically test spkgs.

@vbraun
Copy link
Member

vbraun commented Mar 2, 2012

comment:86

The current workflow is positive review -> release manage tests beta on build bot. While the authors should of course test patches, you don't have to test it on every supported machine to give a positive review.

We did test it on 32 and 6-bit machines which is the most likely source of errors. Specifics for other systems were not changed so its likely that it'll continue to work. I'll give the spell-checked SPKG.txt my positive review.

@jdemeyer
Copy link

jdemeyer commented Mar 3, 2012

comment:87

Replying to @vbraun:

We did test it on 32 and 6-bit machines which is the most likely source of errors.

Agreed. My impression from Paul Zimmermann's comment was that it hadn't been tested at all.

@jdemeyer
Copy link

jdemeyer commented Mar 3, 2012

comment:88

Replying to @vbraun:

We did test it on [...] 6-bit machines

Impressive!

@jdemeyer
Copy link

jdemeyer commented Mar 4, 2012

Changed work issues from typos to none

@jdemeyer
Copy link

jdemeyer commented Mar 4, 2012

Merged: sage-5.0.beta7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants