Upgrade MPFR to 3.1.0

[leif]

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:

• trac_11666_remove_random_hack.patch​
• trac_11666_reduce_precision_max.patch​
• trac_11666_fix_random_doctests.patch​

[leif]

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.

[leif]

The final MPFR 3.1.0 has been released today.

See the description for links.

[mhansen]

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

[fbissey]

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?

[mhansen]

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.

[zimmerma]

I wonder why 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

[zimmerma]

the url from comment 7 does not work.

Paul

[fbissey]

Replying to zimmerma:

I wonder why 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].

[mhansen]

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.

[fbissey]

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]

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 #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 #9522 and #8664)
• make should be replaced by $MAKE in spkg-check

[zimmerma]

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

[zimmerma]

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

Paul

[fbissey]

Replying to zimmerma:

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.

[zimmerma]

on a 32-bit Pentium 4, after applying #12171 and importing this spkg, I get when I start Sage:

ImportError: /localdisk/tmp/sage-4.7.2/local/lib/libmpfi.so.0: undefined symbol: mpfr_add_d
Error importing ipy_profile_sage - perhaps you should run %upgrade?
WARNING: Loading of ipy_profile_sage failed.

Maybe I should first import this spkg then that of MPFI?

Paul

[jpflori]

Still about the spkg itself:

• Is there any reason to "unset RM"? it says it messes with libtool while running "make check". I do not see this restriction in the MPIR package, or the MPFI one e.g. Is this for a specific architecture? Is this really needed? I guess this is fixed by #3537
• MAKE is set back to make before running "make install" (and then "make check" in the original package), any undocumented reason?

I'll test all of this on my system but if someone can point me to the relevant info, I'd be grateful, Google wans not that helpful.

[jpflori]

I've put an updated package implementing my remarks, mainly using the mpir spkg-install file, I've also updated the description and license info.

It can be found at

​http://perso.telecom-paristech.fr/sage/mpfr-3.1.0.spkg

It seems to do what it should, and uccessfully built and rand MPFR test suite on a "usual" intel 64 bit system, not any idea if I broke anything for more exotic architectures. Did not ran sage test suite either.

[leif]

Replying to jpflori:

It can be found at

​http://perso.telecom-paristech.fr/sage/mpfr-3.1.0.spkg

I do get a 301 to ​http://perso.telecom-paristech.fr/~sage/mpfr-3.1.0.spkg and that gives a 404.

[leif]

Replying to jpflori:

Still about the spkg itself:

• Is there any reason to "unset RM"? it says it messes with libtool while running "make check". I do not see this restriction in the MPIR package, or the MPFI one e.g. Is this for a specific architecture? Is this really needed? I guess this is fixed by #3537.

Unsetting RM is safe, but meanwhile a bit redundant. Newer autotools use $RM instead of rm -f (or $RM -f), so tend to fail if RM happens to be set to (just) rm.

• MAKE is set back to make before running "make install" (and then "make check" in the original package), any undocumented reason?

I'm pretty sure that's a relict from when people didn't know how to properly disable parallel make. If in doubt, use $MAKE -j1 install etc.

[jpflori]

Sorry, this should be ​http://perso.telecom-paristech.fr/~flori/sage/mpfr-3.1.0.spkg

[leif]

Replying to zimmerma:

Maybe I should first import this spkg then that of MPFI?

Since MPFI depends on MPFR (and MPIR), you have to rebuild MPFI after upgrading one of the latter.

If you have enough time (and/or the machine is fast enough), you can copy new/updated spkgs into $SAGE_ROOT/spkg/standard/ and run

$ env SAGE_UPGRADING=yes make build

afterwards. That way all dependent packages automatically get rebuilt, which can take some time.

(Unfortunately there are also a couple of stupid transitive dependencies; e.g. ATLAS gets rebuilt if you update readline -- just because ATLAS now has an spkg-install file written in Python, and Python depends on readline... So a lot of rebuilds performed by make aren't really necessary.)

[zimmerma]

I tried importing the MPFI spkg+patches (#12171) and that spkg on top of 4.8.alpha6 on a 32-bit Pentium 4 and got the following failures:

sage -t  "devel/sage-main/sage/matrix/matrix_mpolynomial_dense.pyx"
sage -t  "devel/sage-main/sage/matrix/constructor.py"
sage -t  "devel/sage-main/sage/matrix/matrix2.pyx"
sage -t  "devel/sage-main/sage/misc/randstate.pyx"
sage -t  "devel/sage-main/sage/modules/free_module_element.pyx"
sage -t  "devel/sage-main/sage/rings/real_mpfi.pyx"
sage -t  "devel/sage-main/sage/rings/complex_field.py"
sage -t  "devel/sage-main/sage/rings/complex_interval_field.py"
sage -t  "devel/sage-main/sage/rings/real_mpfr.pyx" # Killed/crashed
sage -t  "devel/sage-main/sage/schemes/elliptic_curves/ell_rational_field.py"

Paul

[zimmerma]

the failures are most likely due to the fact that the mpfr_urandom and mpfr_urandomb return identical values on processors with different word size in MPFR 3.1.0 (see ​http://www.mpfr.org/mpfr-3.1.0/#changes).

The bad news is that this will need changing the doctests.

The good news is that we won't need any more different doctests on 32-bit and 64-bit (assuming the MPIR random generator is independent of the word size).

Paul Zimmermann

[jpflori]

For the record, Mike's spkg and my updated one both need to be rebased on top of #12131.

I'll take care of that later.

[jpflori]

You can finally find a rebased spkg where I discarded some stuff involving ABI which previously made sage print that it was building for 32 bits, whereas it actually had no effect, and which is rebased on #12131 at the usual address:

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

I guess the issue with doctests is the last step before "needs review"

[jpflori]

I guess my last package should be rebased on #12366 which has been merged into 5.beta4.

Paul, any news on the doctests problems ?

[zimmerma]

I will check the doctests on cicero (skynet) but I guess the issue is still there, since nothing was changed. I will try to make a patch (but since I'm in holidays, it might take some time).

Paul

[zimmerma]

I tried this patch (with dependencies) on top of Sage 4.8 and got several doctests failures:

        sage -t  devel/sage-11666/sage/schemes/elliptic_curves/ell_rational_field.py # 2 doctests failed
        sage -t  devel/sage-11666/sage/schemes/elliptic_curves/heegner.py # 2 doctests failed
        sage -t  devel/sage-11666/sage/matrix/matrix2.pyx # 3 doctests failed
        sage -t  devel/sage-11666/sage/combinat/partition.py # 30 doctests failed
        sage -t  devel/sage-11666/sage/calculus/calculus.py # 7 doctests failed
        sage -t  devel/sage-11666/sage/misc/randstate.pyx # 13 doctests failed
        sage -t  devel/sage-11666/sage/misc/misc.py # 1 doctests failed
        sage -t  devel/sage-11666/sage/modules/free_module_element.pyx # 2 doctests failed
        sage -t  devel/sage-11666/sage/rings/real_mpfr.pyx # 12 doctests failed
        sage -t  devel/sage-11666/sage/matrix/constructor.py # 2 doctests failed
        sage -t  devel/sage-11666/sage/combinat/sloane_functions.py # 5 doctests failed
        sage -t  devel/sage-11666/sage/misc/cachefunc.py # 14 doctests failed
        sage -t  devel/sage-11666/sage/combinat/combinat.py # 1 doctests failed
        sage -t  devel/sage-11666/sage/libs/fplll/fplll.pyx # Killed/crashed
        sage -t  devel/sage-11666/sage/combinat/species/species.py # 3 doctests failed
        sage -t  devel/sage-11666/sage/combinat/species/sum_species.py # 3 doctests failed
        sage -t  devel/sage-11666/sage/combinat/species/permutation_species.py # 3 doctests failed
        sage -t  devel/sage-11666/sage/sets/disjoint_union_enumerated_sets.py # 8 doctests failed
        sage -t  devel/sage-11666/sage/matrix/matrix_mpolynomial_dense.pyx # 2 doctests failed
        sage -t  devel/sage-11666/sage/combinat/species/partition_species.py # 4 doctests failed
        sage -t  devel/sage-11666/sage/combinat/species/product_species.py # 2 doctests failed
        sage -t  devel/sage-11666/sage/rings/complex_field.py # 4 doctests failed
        sage -t  devel/sage-11666/sage/rings/complex_interval_field.py # 3 doctests failed
        sage -t  devel/sage-11666/sage/combinat/partitions.pyx # 1 doctests failed
        sage -t  devel/sage-11666/sage/rings/real_mpfi.pyx # 5 doctests failed

Can someone reproduce that?

Paul

[jpflori]

I've begun the test suite. Here are my failures on a Debian amd64 experimental system (on intel core i7) :

The following tests failed:

	sage -t  -force_lib devel/sage/sage/modular/modsym/ambient.py # 0 doctests failed
	sage -t  -force_lib devel/sage/sage/modular/hecke/hecke_operator.py # 0 doctests failed
	sage -t  -force_lib devel/sage/sage/modular/hecke/ambient_module.py # 0 doctests failed
	sage -t  -force_lib devel/sage/sage/modules/free_module_element.pyx # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/matrix_mpolynomial_dense.pyx # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/constructor.py # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/matrix2.pyx # 3 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/real_mpfi.pyx # 5 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/complex_field.py # 4 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/real_mpfr.pyx # 12 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/complex_interval_field.py # 3 doctests failed
	sage -t  -force_lib devel/sage/sage/tests/cmdline.py # 4 doctests failed
	sage -t  -force_lib devel/sage/sage/misc/randstate.pyx # 13 doctests failed
	sage -t  -force_lib devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py # 2 doctests failed

I'll have a look at them tomorrow.

Some of them at least are just changes in random output (e. g. real_mpfr and real_mpfi)

[jpflori]

Some other look more problematic:

sage -t  -force_lib devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py
  ***   Warning: new stack size = 1003360 (0.957 Mbytes).
  ***   Warning: new stack size = 1003360 (0.957 Mbytes).
Saturation index bound = 265
WARNING: saturation at primes p > 97 will not be done;  
points may be unsaturated at primes between 97 and index bound
Failed to saturate MW basis at primes [ ]
Saturation index bound = 265
WARNING: saturation at primes p > 199 will not be done;  
points may be unsaturated at primes between 199 and index bound
Failed to saturate MW basis at primes [ ]
After 10 attempts at enlargement, giving up!
--points not proved 2-saturated,
2-saturation failed!
Failed to saturate MW basis at primes [ 2 ]
2 After 10 attempts at enlargement, giving up!
--points not proved 2-saturated,
2-saturation failed!
Failed to saturate MW basis at primes [ 2 ]

Maybe because I did not rebuild Pari spkg?

Apart from running sage -b I just forced rebuild of libfplll before the above make ptest (because of lack of time).

I'll properly rebuild everything tomorrow and rerun the testsuite.

[jpflori]

Nevermind, the above is potentially just verbosity and not related to the actual errors

File "/home/jp/boulot/sage/sage-4.8/devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py", line 2921:
    sage: 2 * E.elliptic_exponential(z)
Expected:
    (2.04119347066305 - 1.10251372205166*I : 2.23105000976838 - 2.69795281735238*I : 1.00000000000000)
Got:
    (-1.52184235874404 - 0.0581413944316544*I : 0.948655866506124 - 0.0381469928565030*I : 1.00000000000000)
**********************************************************************
File "/home/jp/boulot/sage/sage-4.8/devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py", line 2923:
    sage: E.elliptic_exponential(2 * z)
Expected:
    (2.04119347066305 - 1.10251372205167*I : 2.23105000976839 - 2.69795281735236*I : 1.00000000000000)
Got:
    (-1.52184235874404 - 0.0581413944316562*I : 0.948655866506128 - 0.0381469928565034*I : 1.00000000000000)
**********************************************************************

And the line above defining z implies random so...

[jpflori]

On a different system (Debian amd64 experimental on an intel QuadCore2) I get

The following tests failed:

	sage -t  -force_lib devel/sage/sage/modules/free_module_element.pyx # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/real_mpfi.pyx # 5 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/real_mpfr.pyx # 12 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/complex_interval_field.py # 3 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/complex_field.py # 4 doctests failed
	sage -t  -force_lib devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/constructor.py # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/matrix2.pyx # 3 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/matrix_mpolynomial_dense.pyx # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/misc/randstate.pyx # 13 doctests failed
----------------------------------------------------------------------

This also is on top of 4.8, but I could not launch sage before also rebuilding MPIR which was not necessary on the other system.

[jpflori]

Replying to zimmerma:

I tried importing the MPFI spkg+patches (#12171) and that spkg on top of 4.8.alpha6 on a 32-bit Pentium 4 and got the following failures: sage -t "devel/sage-main/sage/matrix/matrix_mpolynomial_dense.pyx" sage -t "devel/sage-main/sage/matrix/constructor.py" sage -t "devel/sage-main/sage/matrix/matrix2.pyx" sage -t "devel/sage-main/sage/misc/randstate.pyx" sage -t "devel/sage-main/sage/modules/free_module_element.pyx" sage -t "devel/sage-main/sage/rings/real_mpfi.pyx" sage -t "devel/sage-main/sage/rings/complex_field.py" sage -t "devel/sage-main/sage/rings/complex_interval_field.py" sage -t "devel/sage-main/sage/rings/real_mpfr.pyx" # Killed/crashed sage -t "devel/sage-main/sage/schemes/elliptic_curves/ell_rational_field.py" Paul

My last failures happen in the same files as what you posted some time ago and are always involving some random function.

As you posted later, this should be because MPFR changed its behavior.

I'll post a patch fixing the doctest, but cannot test it on a 32 bits computer (nor exotic architectures), so I'll leave that to someone else.

The question is whether this change of behavior is actually a problem ?

If someone saved old stuff with a given random seed, or something like that, its results will not be reproducible... I do not really feel concerned about that.

If someone could also test the spkg on sunos, solaris, apple or other exotic oses/cpus combinations, that would be more than welcome.

[jpflori]

After rebuilding mpir and atlas (which was screwed) on my core i7 setup, here is what I get

----------------------------------------------------------------------
The temporary doctesting directory
   /home/jp/.sage/tmp/jp_x220-9822
was not removed: it is not empty, presumably because doctests
failed or doctesting was interrupted.

----------------------------------------------------------------------

The following tests failed:

	sage -t  -force_lib devel/sage/sage/modular/modsym/space.py # 0 doctests failed
	sage -t  -force_lib devel/sage/sage/modules/free_module_element.pyx # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/matrix_mpolynomial_dense.pyx # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/constructor.py # 2 doctests failed
	sage -t  -force_lib devel/sage/sage/matrix/matrix2.pyx # 3 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/real_mpfi.pyx # 5 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/complex_field.py # 4 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/real_mpfr.pyx # 12 doctests failed
	sage -t  -force_lib devel/sage/sage/rings/complex_interval_field.py # 3 doctests failed
	sage -t  -force_lib devel/sage/sage/tests/cmdline.py # 4 doctests failed
	sage -t  -force_lib devel/sage/sage/misc/randstate.pyx # 13 doctests failed
	sage -t  -force_lib devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py # 2 doctests failed

I guess something went wrong with that installation because the failing doctest in cmdline.pyx really look strange.

[zimmerma]

Replying to leif:

If you have enough time (and/or the machine is fast enough), you can copy new/updated spkgs into $SAGE_ROOT/spkg/standard/ and run

$ env SAGE_UPGRADING=yes make build

afterwards. That way all dependent packages automatically get rebuilt, which can take some time.

should I rename new packages with the old name? Or keep the new name? For example Sage 4.8 contains mpfi-1.3.4-cvs20071125.p9.spkg. Should I put mpfi-1.5.1.spkg aside this one, or rename it?

Paul

[zimmerma]

I just ran the doctests on cicero (skynet) with vanilla 4.8 and all of them pass.

Paul

[jpflori]

Just to confirm my above statements, on a third amd64 ubuntu system, I get the same errors as above, except the two "strange" ones, i.e. cmdline.py and space.py.

[jpflori]

And those last tests were on top sage-5.0.beta4

[jpflori]

I've had a look at the random MPFR real number generation code and there was indeed a "hack" for 32 bits systems before which requested more bits (31 from randstate doc, although real_mpfr says 32) from MPIR to get an equivalent MPFR behavior. Although, if I force sage to do the same on my 64 bits system (just adding a call to rstate.c_random() unconditionally in real_mpfr), I do not get what 32 bits (and 64 bits) systems used to return (nor what my 64 bits system now return which was to expect).

Paul, I'm confused by your last message. By vanilla 4.8, do you mean vanilla+this ticket and dependencies? If that's the case, and I somehow understand correctly the random generation stuff of MPIR and MPFR which should both be hardware independent now, the change I made above should give me a similar behavior as you on a 32 bits system and get no doctest failures.

For example the last failing test in real_mpfr used to give

0.979095507956490

With this ticket and dependencies, I get (on all of my 64 systems)

-0.616678906367394

With the additional call to c_random() I get (on all of my 64 systems)

0.681934947736557

[zimmerma]

Paul, I'm confused by your last message. By vanilla 4.8, do you mean vanilla+this ticket and dependencies?

no, I just meant vanilla 4.8. This was just to make sure that potential failing doctests did not fail before this patch.

Now with #12171 all doctests still pass (with one timeout even with -long, for a test taking more than half an hour).

I'll now try with this spkg.

Paul

[jpflori]

Replying to zimmerma:

no, I just meant vanilla 4.8. This was just to make sure that potential failing doctests did not fail before this patch.

That's good news for the tests I ran above. I guess the piece of hack in real_mpfr should now be removed. Could you try running the tests with and without the c_random() stuff in real_mpfr and post the values you get for the last failing (I guess it will fail as well) in real_mpfr (on 32 bits I mean)?

The bad news is that we won't be able (or it will be very hackish???) to reproduce the previous behavior of Sage, thus breaking all use code depending on the deterministic behavior of PRNG.

[zimmerma]

Could you try running the tests with and without the c_random() stuff in real_mpfr [...]

ok, I'll first run the tests with the attached spkg. Currently running env SAGE_UPGRADING=yes make build, this will take some time since cicero is slow.

Paul

[zimmerma]

Jean-Pierre,

I've had a look at the random MPFR real number generation code and there was indeed a "hack" for 32 bits systems before which requested more bits (31 from randstate doc, although real_mpfr says 32) from MPIR to get an equivalent MPFR behavior. Although, if I force sage to do the same on my 64 bits system (just adding a call to rstate.c_random() unconditionally in real_mpfr), I do not get what 32 bits (and 64 bits) systems used to return (nor what my 64 bits system now return which was to expect).

this is expected. If you add the call to rstate.c_random() unconditionally, then on 64-bit systems you will draw 64 extra random bits when the precision mod 64 is between 1 and 32, whereas on 32-bit systems you will draw only 32 extra random bits.

In fact, since version 3.1.0, MPFR leaves the GMP random generator in the same state, whatever the number of bits per limb. See changeset 7133 from MPFR, and ​http://gmplib.org/list-archives/gmp-devel/2010-September/001642.html. Thus the fix is simply to remove the "gross hack" at lines 930-942 in real_mpfr.pyx.

Paul

[jpflori]

From what i saw in randstate, c_random calls gmp_urandomb_ui(self.gmp_state, 31).

So this has different behaviors on the gmp rand internal state on 32 and 64 bits ?

I expected that it didn't, so that adding a call to c_random() even on 64 bits (just to see, I don't plan on integrating it to Sage, removing the hack is indeed the right solution) would produce identical behaviors on 32 and 64 bits, whence my request for you to test it (in fact with the hack and without it, not with a call to c_random added, on 32 bits).

I also had little hope that it could revert the real PNRG to its previous behavior for a given seed (in order not to break existing code, outside Sage source tree I mean).

Anyway, if the mpfr code changed, its completely understandable that the random output changes, even with the same seed, so this is hopeless and not really harmful (to me at least).

Anyway, if you confirm that you get the same values that I get (with no call to c_random at all, ie without the hackish section only running on 32 bits currently), I'll post patches to remove the hack section and correct the doctests.

[zimmerma]

From what i saw in randstate, c_random calls gmp_urandomb_ui(self.gmp_state, 31). So this has different behaviors on the gmp rand internal state on 32 and 64 bits ?

no it shouldn't. But beware that Sage uses MPIR, not GMP. We should check that MPIR gives the same behaviour for the random state on 32-bit and 64-bit processors.

I'll tell you what I get as soon as make build finished...

Paul

[jpflori]

The files randbui.c where gmp_urandomb_ui is defined are similar in GMP (5.04) and MPIR (2.1.3.p9 in Sage).

They only call _gmp_rand defined in gmp-impl.h which looks quite similar as well (in fact I saw no diff but did not check completely, there could be some black magic somewhere deep enough).

From what I see in integer_ring.pyx and more specifically in the _randomize_mpz function where c_random is called as well, there is no different sections for 32 and 64 bits, nor different doctest in randstate.pyx (only rgp from Pari does change), so I guess it is not that unsafe to assume that MPIR behaves the same for both.

[zimmerma]

Jean-Pierre,

the test of real_mpfr failed with a Segmentation fault on cicero. However I get the following with the "gross hack" on cicero (32-bit Pentium 4):

----------------------------------------------------------------------
| Sage Version 4.8, Release Date: 2012-01-20                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: R=RealField(65)
sage: set_random_seed(42)
sage: R.random_element()
0.9396630990748764761
sage: R.random_element()
0.2987261287446042432

and without the "gross hack":

----------------------------------------------------------------------
| Sage Version 4.8, Release Date: 2012-01-20                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: R=RealField(65)
sage: set_random_seed(42)
sage: R.random_element()
0.7846023201112678858
sage: R.random_element()
0.3289891249716495404

You should be able to reproduce the later on a 64-bit machine.

Paul

[jpflori]

Indeed.

When forcing the call to c_random(), I get

----------------------------------------------------------------------
| Sage Version 4.8, Release Date: 2012-01-20                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
Loading Sage library. Current Mercurial branch is: mpfr
sage: R=RealField(65)
sage: set_random_seed(42)
sage: R.random_element()
0.9396630990748764761
sage: R.random_element()
0.2987261287446042432

And with "normal" behavior:

----------------------------------------------------------------------
| Sage Version 4.8, Release Date: 2012-01-20                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
Loading Sage library. Current Mercurial branch is: mpfr
sage: R=RealField(65)    
sage: set_random_seed(42)
sage: R.random_element() 
0.7846023201112678858
sage: R.random_element() 
0.3289891249716495404

So everything looks fine, except for the segfault you got.

Hopefully, I'll provide patches for removing the hackish section and fixing the doctests tomorrow.

[vbraun]

We want to work on the FLINT update at Sage Days 36 so I'll give this a try. I have access to a i7-920 running Linux i386 so it would be easy to test.

[vbraun]

The segfault is from

sage: R = RealField(2147483647)

because MPFR contains code like

        _srcs  = (_srcprec  + GMP_NUMB_BITS-1)/GMP_NUMB_BITS;         
        _dests = (_destprec + GMP_NUMB_BITS-1)/GMP_NUMB_BITS - _srcs; 

This gives an int overflow when prec=INT_MAX, and soon after as segfault because the buffer sizes are wrong.

[vbraun]

Initial patch

[vbraun]

Initial patch

[vbraun]

I think the easiest solution is to limit the maximal precision in Sage to be slightly below 32-bit INT_MAX. The attached patches do that.

[vbraun]

Reported upstream at ​https://gforge.inria.fr/tracker/index.php?func=detail&aid=13918&group_id=136&atid=619

[vbraun]

I ran the doctests on 32-bit and 64-bit machines and they work.

[jpflori]

As I commented above, the current package needs to be rebased on #12366.

I'm doing it right now. I'll also fix the MPFI package so everything can finally be merged.

I had a look at your patches and have nothing to say :) So once I upload the new spkg, I guess it will be positive_review.

[jpflori]

Updated spkg is available at ​http://perso.telecom-paristech.fr/~flori/sage/mpfr-3.1.0.p0.spkg

[jpflori]

The above link doesn't point to the right file when clicked...

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

This one should be ok.

[jpflori]

All test passed on my amd64 system.

If someone could have a final look at the updated spkg, I guess this can be finally set as positive_review.

[vbraun]

The Solaris patch is applied to the wrong directory, it is currenty doing cp ../patches/mpn_exp.c mpn_exp.c but if you want to overwrite the MPFR source then you should do cp ../patches/mpn_exp.c src/mpn_exp.c (there is a src/src directory in MPFR-3.1.0, this was just src/ in the old MPFR). Can you fix this as well?

[vbraun]

Too fast with positive review ;-)

[zimmerma]

I wonder why some random tests still differ between 32-bit and 64-bit computers. Is there any reason?

Paul

[zimmerma]

I tried the patches on top of Sage 4.8 (after #12171) but the 2nd and 3rd failed to apply:

sage: hg_sage.import_patch("trac_11666_reduce_precision_max.patch")
cd "/home/zimmerma/sage-4.8/devel/sage" && sage --hg import   "/home/zimmerma/sage-4.8/trac_11666_reduce_precision_max.patch"
applying /home/zimmerma/sage-4.8/trac_11666_reduce_precision_max.patch
patching file sage/rings/real_mpfr.pyx
Hunk #1 FAILED at 183
1 out of 1 hunks FAILED -- saving rejects to file sage/rings/real_mpfr.pyx.rej
abort: patch failed to apply

and

sage: hg_sage.import_patch("trac_11666_fix_random_doctests.patch") 
cd "/home/zimmerma/sage-4.8/devel/sage" && sage --hg import   "/home/zimmerma/sage-4.8/trac_11666_fix_random_doctests.patch"
applying /home/zimmerma/sage-4.8/trac_11666_fix_random_doctests.patch
patching file sage/misc/randstate.pyx
Hunk #1 FAILED at 54
Hunk #2 FAILED at 85
Hunk #3 FAILED at 221
Hunk #4 FAILED at 233
Hunk #5 FAILED at 254
Hunk #6 FAILED at 276
6 out of 6 hunks FAILED -- saving rejects to file sage/misc/randstate.pyx.rej
abort: patch failed to apply

Paul

[jpflori]

Replying to ​zimmerma:

I wonder why some random tests still differ between 32-bit and 64-bit computers. Is there any reason? Paul

You mean in randstate.pyx? I guess that Pari RNG does not behaves the same on 32 and 64 bits. The other tests (MPIR, MPFR, GAP, NTL, Python) looks the same to me, but i might have missed something.

The spkg at the usual address is updated, if someone could test that the patch now applies on Solaris :)

For Paul: the Sage patches of the ticker did apply for me on top of 5.0.beta4

[zimmerma]

we decided in the MPFR development version to decrease the maximal value of the precision from 256, i.e., it is now 2³¹-257. You might want to do the same in Sage, so that upgrading to future MPFR versions will not cause any problem.

Paul

[vbraun]

Updated patch to limit precision at 231-257 Updated patch to limit precision at 231-257 Updated patch to limit precision at 231-257

[vbraun]

Looks good. I'm happy to give the spkg a positive review. Nothing changed in mpn_exp.c except the file location, so I don't foresee any Solaris issues.

I've implemented Paul's suggestion about the precision limit, which was the only reviewer complaint. So I'll take it that everyone agrees with positive review ;-)

Patches apply cleanly against sage-5.0.alpha4 and alpha5.

[jdemeyer]

Sorry for the trouble, but this should be rebased to #12548.

[jpflori]

Here you go.

Same address as usual.

[jdemeyer]

SPKG.txt refers incorrectly to mpfr-3.1.0.p1, it should be .p0

[jpflori]

Corrected.

[vbraun]

Looks good!

[zimmerma]

I'm not sure my advice in comment 11 to configure MPFR with -disable-thread-safe was taken into account. This might cause problems on various systems, see for example ​http://www.loria.fr/~zimmerma/software/compilerbugs.html

In addition, I'm not sure MPFR can be used as multi-thread within Sage.

Paul

[jpflori]

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]

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 ;-)

[zimmerma]

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]

SPKG updated at the same address.

[jpflori]

Is there anything preventing this from getting positive review ?

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

[zimmerma]

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]

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]

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

The package is at the same address as before.

[jpflori]

spkg diff, for review only

[jdemeyer]

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]

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]

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]

Replying to vbraun:

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

Impressive!