sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 486 forks source link

upgrade arb to 2.12.0 #24369

Closed edd8e884-f507-429a-b577-5d554626c0fe closed 6 years ago

edd8e884-f507-429a-b577-5d554626c0fe commented 6 years ago

arb fails to pass self-tests on x86_64 skylake. This is fixed in arb 2.12.0.

(The new version fails to pass self-tests on my 32bit VM, while it did not before, see #24661.)

tarball here: https://github.com/fredrik-johansson/arb/archive/2.12.0.tar.gz

Upstream: Fixed upstream, in a later stable release.

CC: @fredrik-johansson @kiwifb

Component: packages: standard

Keywords: sdl

Author: Dima Pasechnik

Branch: c793138

Reviewer: Thierry Monteil, Marc Mezzarobba, Fredrik Johansson

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

edd8e884-f507-429a-b577-5d554626c0fe commented 6 years ago
comment:1

Attachment: arb-2.11.1.p0.log

edd8e884-f507-429a-b577-5d554626c0fe commented 6 years ago

Upstream: Reported upstream. No feedback yet.

fredrik-johansson commented 6 years ago
comment:2

Does upgrading to 2.12 solve the issue?

dimpase commented 6 years ago
comment:3

By the way, the same error on x86_64 (a skylake CPU, Linux)

[arb-2.11.1.p0] gauss_period_minpoly....make[3]: *** [../Makefile.subdirs:84: 
../build/arb_fmpz_poly/test/t-gauss_period_minpoly_RUN] Floating point exception

I'll investigate the upgrade option...

dimpase commented 6 years ago

Author: Dima Pasechnik

dimpase commented 6 years ago

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, in a later stable release.

dimpase commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,2 +1,5 @@
 arb fails to pass self-tests on my 32bit VM, while it did not before. Log attached.

+Fixed in arb 2.12.0.
+
+tarball here: https://github.com/fredrik-johansson/arb/archive/2.12.0.tar.gz
dimpase commented 6 years ago

Commit: c793138

dimpase commented 6 years ago
comment:4

mildly tested on x86_64, fixed numerical noise (which almost uniformly went down :-)).


New commits:

c793138update arb to 2.12.0, doctest fixes, and #24369
dimpase commented 6 years ago

Branch: u/dimpase/arb212

kiwifb commented 6 years ago
comment:6

Not a good time for me. I won't be able to give you a review before the 3rd or 4rth of January.

edd8e884-f507-429a-b577-5d554626c0fe commented 6 years ago
comment:7

With 2.12.0, i still have one test failure on 32bit system:

l_vec_hurwitz....
 L value differ
L(1/2, 1) single = (0 + 0j)  +/-  (0, 0j)
L(1/2, 1) multi = (-0.30909754859579756426 + 0j)  +/-  (2.54e-23, 0j)

(-0.3090975486 + 0j)  +/-  (2.54e-23, 0j)
(0.7888523833 - 0.1836680212j)  +/-  (2.02e-23, 1.54e-23j)
(0.4432168971 - 0.236477911j)  +/-  (1.99e-23, 1.32e-23j)
(1.648021942 + 0.03777565741j)  +/-  (1.55e-23, 8.22e-24j)
(0.6185977148 + 0.146695673j)  +/-  (2.08e-23, 1.32e-23j)
(0.5039871966 - 0.8215955013j)  +/-  (2.02e-23, 1.71e-23j)
(0.750432146 + 0j)  +/-  (2.54e-23, 0j)
(0.5039871966 + 0.8215955013j)  +/-  (2.02e-23, 1.71e-23j)
(0.6185977148 - 0.146695673j)  +/-  (2.08e-23, 1.32e-23j)
(1.648021942 - 0.03777565741j)  +/-  (1.55e-23, 8.22e-24j)
(0.4432168971 + 0.236477911j)  +/-  (1.99e-23, 1.32e-23j)
(0.7888523833 + 0.1836680212j)  +/-  (2.02e-23, 1.54e-23j)

../Makefile.subdirs:84: recipe for target '../build/acb_dirichlet/test/t-l_vec_hurwitz_RUN' failed

See also the attached log.

edd8e884-f507-429a-b577-5d554626c0fe commented 6 years ago

Reviewer: Thierry Monteil

dimpase commented 6 years ago
comment:8

Attachment: arb-2.12.0.p0.log

while I can confirm that this is reproducible on a "real" x86 (arando buildbot)

[arb-2.12.0.p0] l_vec_hurwitz....
[arb-2.12.0.p0]  L value differ
[arb-2.12.0.p0] L(1/2, 1) single = (0 + 0j)  +/-  (0, 0j)
[arb-2.12.0.p0] L(1/2, 1) multi = (-0.30909754859579756426 + 0j)  +/-  (2.54e-23, 0j)
[arb-2.12.0.p0]
[arb-2.12.0.p0] (-0.3090975486 + 0j)  +/-  (2.54e-23, 0j)
[arb-2.12.0.p0] (0.7888523833 - 0.1836680212j)  +/-  (2.02e-23, 1.54e-23j)
[arb-2.12.0.p0] (0.4432168971 - 0.236477911j)  +/-  (1.99e-23, 1.32e-23j)
[arb-2.12.0.p0] (1.648021942 + 0.03777565741j)  +/-  (1.55e-23, 8.22e-24j)
[arb-2.12.0.p0] (0.6185977148 + 0.146695673j)  +/-  (2.08e-23, 1.32e-23j)
[arb-2.12.0.p0] (0.5039871966 - 0.8215955013j)  +/-  (2.02e-23, 1.71e-23j)
[arb-2.12.0.p0] (0.750432146 + 0j)  +/-  (2.54e-23, 0j)
[arb-2.12.0.p0] (0.5039871966 + 0.8215955013j)  +/-  (2.02e-23, 1.71e-23j)
[arb-2.12.0.p0] (0.6185977148 - 0.146695673j)  +/-  (2.08e-23, 1.32e-23j)
[arb-2.12.0.p0] (1.648021942 - 0.03777565741j)  +/-  (1.55e-23, 8.22e-24j)
[arb-2.12.0.p0] (0.4432168971 + 0.236477911j)  +/-  (1.99e-23, 1.32e-23j)
[arb-2.12.0.p0] (0.7888523833 + 0.1836680212j)  +/-  (2.02e-23, 1.54e-23j)
[arb-2.12.0.p0]
[arb-2.12.0.p0]
[arb-2.12.0.p0] make[3]: *** [../build/acb_dirichlet/test/t-l_vec_hurwitz_RUN] Aborted (core dumped)

I still insist that we should upgrade, as the previous version also does not pass all the self-tests, albeit on a different (and much more popular) platform, x86_64 (if it is skylake- it seems to work OK on nechalem).

See the attached log.

dimpase commented 6 years ago

Attachment: arb-2.11.1.p0.2.log

failure on skylake

dimpase commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,5 +1,5 @@
-arb fails to pass self-tests on my 32bit VM, while it did not before. Log attached.
+arb fails to pass self-tests on my 32bit VM, while it did not before. Log attached. As well, it fails to pass self-tests on x86_64 skylake.

-Fixed in arb 2.12.0.
+The latter is fixed in arb 2.12.0.

 tarball here: https://github.com/fredrik-johansson/arb/archive/2.12.0.tar.gz
kiwifb commented 6 years ago
comment:10

Just to make sure, your commit remove the whole of src/sage/rings/complex_arb.pyx but not the corresponding pxd file? are you sure?

dimpase commented 6 years ago
comment:11

Replying to @kiwifb:

Just to make sure, your commit remove the whole of src/sage/rings/complex_arb.pyx but not the corresponding pxd file? are you sure?

this is a trac git viewer bug. If you look at the commit in comment 4, it would make sense.

kiwifb commented 6 years ago
comment:12

Replying to @dimpase:

Replying to @kiwifb:

Just to make sure, your commit remove the whole of src/sage/rings/complex_arb.pyx but not the corresponding pxd file? are you sure?

this is a trac git viewer bug. If you look at the commit in comment 4, it would make sense.

Nope, commit in comment 4 looks just the same.

dimpase commented 6 years ago
comment:13

Huh? https://github.com/sagemath/sagetrac-mirror/commit/c79313811851584c5b6732ea12da8b40d04b0377

(it could be it is different for you). To be sure, use git, or look here: https://github.com/sagemath/sagetrac-mirror/compare/u/dimpase/arb212

dimpase commented 6 years ago
comment:14

This is how it looks to me:


author  Dima Pasechnik <dimpase@gmail.com>  2017-12-11 23:58:06 +0000
committer   Dima Pasechnik <dimpase@gmail.com>  2017-12-11 23:58:06 +0000
commit  c79313811851584c5b6732ea12da8b40d04b0377 (patch)
tree    94c8774bc96b6b9e5386f9e5502ee60db3de0eb2
parent  Updated SageMath version to 8.1 (diff)
update arb to 2.12.0, doctest fixes, and #24369u/dimpase/arb212
Diffstat
-rw-r--r--  build/pkgs/arb/checksums.ini    6   
-rw-r--r--  build/pkgs/arb/package-version.txt  2   
-rw-r--r--  src/sage/rings/complex_arb.pyx  14  
-rw-r--r--  src/sage/rings/real_arb.pyx 4   
4 files changed, 13 insertions, 13 deletions
diff --git a/build/pkgs/arb/checksums.ini b/build/pkgs/arb/checksums.ini
index 78909d5..1924ee0 100644
--- a/build/pkgs/arb/checksums.ini
+++ b/build/pkgs/arb/checksums.ini
@@ -1,4 +1,4 @@
 tarball=arb-VERSION.tar.gz
-sha1=2f06bfb433cdaecde0e824c5e638094fd666a0d1
-md5=d63cdd1147104790826c93bc8651104f
-cksum=2745482665
+sha1=27476d0529e48a07d92da90bd0fb80dd18f443e3
+md5=733285d9705d10b8024e551ffa81952f
+cksum=2391183744
diff --git a/build/pkgs/arb/package-version.txt b/build/pkgs/arb/package-version.txt
index 99993ff..c8810e9 100644
--- a/build/pkgs/arb/package-version.txt
+++ b/build/pkgs/arb/package-version.txt
@@ -1 +1 @@
-2.11.1.p0
+2.12.0.p0
diff --git a/src/sage/rings/complex_arb.pyx b/src/sage/rings/complex_arb.pyx
index ee048fb..71fad24 100644
--- a/src/sage/rings/complex_arb.pyx
+++ b/src/sage/rings/complex_arb.pyx
@@ -2603,7 +2603,7 @@ cdef class ComplexBall(RingElement):
         EXAMPLES::

             sage: CBF(0, -1).agm1()
-            [0.5990701173678 +/- 1.15e-14] + [-0.5990701173678 +/- 1.19e-14]*I
+            [0.599070117367796 +/- 3.9...e-16] + [-0.599070117367796 +/- 5.5...e-16]*I
         """
         cdef ComplexBall res = self._new()
         if _do_sig(prec(self)): sig_on()
@@ -3318,9 +3318,9 @@ cdef class ComplexBall(RingElement):
              [0.002473055794309 +/- 5.01e-16] + [0.003859554040267 +/- 4.45e-16]*I,
              [-0.01299087561709 +/- 4.72e-15] + [0.00725027521915 +/- 4.32e-15]*I]
             sage: (z + 3 + 4*tau).elliptic_p(tau, 3)
-            [[-3.2892099677271 +/- 2.29e-14] + [-0.00036737673029 +/- 8.58e-15]*I,
-             [0.002473055794 +/- 6.59e-13] + [0.003859554040 +/- 6.17e-13]*I,
-             [-0.0129908756 +/- 3.39e-11] + [0.0072502752 +/- 3.60e-11]*I]
+            [[-3.28920996772709 +/- 8.4...e-15] + [-0.00036737673029 +/- 4.1...e-15]*I,
+             [0.0024730557943 +/- 6.6...e-14] + [0.0038595540403 +/- 8.8...e-14]*I,
+             [-0.01299087562 +/- 5.6...e-12] + [0.00725027522 +/- 3.5...e-12]*I]

         """
         cdef ComplexBall my_tau = self._parent.coerce(tau)
@@ -3356,7 +3356,7 @@ cdef class ComplexBall(RingElement):
         EXAMPLES::

             sage: CBF(2,3).elliptic_k()
-            [1.0429132919285 +/- 3.65e-14] + [0.6296824723086 +/- 6.15e-14]*I
+            [1.04291329192852 +/- 5.9...e-15] + [0.62968247230864 +/- 3.4...e-15]*I

         """
         cdef ComplexBall result = self._new()
@@ -3373,7 +3373,7 @@ cdef class ComplexBall(RingElement):
         EXAMPLES::

             sage: CBF(2,3).elliptic_e()
-            [1.472797144959 +/- 4.82e-13] + [-1.231604783936 +/- 1.25e-13]*I
+            [1.472797144959 +/- 4.5...e-13] + [-1.231604783936 +/- 9.5...e-14]*I

         """
         cdef ComplexBall result = self._new()
@@ -3520,7 +3520,7 @@ cdef class ComplexBall(RingElement):
         EXAMPLES::

             sage: CBF(1/2).legendre_P(5)
-            0.08984375000000000
+            [0.08984375000000000 +/- 4.5...e-18]
             sage: CBF(1,2).legendre_P(CBF(2,3), CBF(0,1))
             [0.10996180744364 +/- 7.45e-15] + [0.14312767804055 +/- 8.38e-15]*I
             sage: CBF(-10).legendre_P(5, 325/100)
diff --git a/src/sage/rings/real_arb.pyx b/src/sage/rings/real_arb.pyx
index aa12ae0..9aad425 100644
--- a/src/sage/rings/real_arb.pyx
+++ b/src/sage/rings/real_arb.pyx
@@ -3451,7 +3451,7 @@ cdef class RealBall(RingElement):
             sage: RBF(1/2).polylog(1)
             [0.6931471805599 +/- 5.02e-14]
             sage: RBF(1/3).polylog(1/2)
-            [0.44210883528067 +/- 6.75e-15]
+            [0.44210883528067 +/- 6.7...e-15]
             sage: RBF(1/3).polylog(RLF(pi))
             [0.34728895057225 +/- 5.51e-15]

@@ -3551,7 +3551,7 @@ cdef class RealBall(RingElement):
             sage: RBF(1).agm(1)
             1.000000000000000
             sage: RBF(sqrt(2)).agm(1)^(-1)
-            [0.83462684167407 +/- 4.31e-15]
+            [0.83462684167407 +/- 3.9...e-15]
         """
         cdef RealBall other_as_ball
         cdef RealBall res = self._new()
kiwifb commented 6 years ago
comment:15

Oh yes that's right. I did something wrong there.

kiwifb commented 6 years ago
comment:16

I was going to give this a positive review but re-reading things I am getting concerned that the new arbś tests fail systematically on 32bits machine. Is it a correct assertion?

edd8e884-f507-429a-b577-5d554626c0fe commented 6 years ago
comment:17

Replying to @kiwifb:

I was going to give this a positive review but re-reading things I am getting concerned that the new arbś tests fail systematically on 32bits machine. Is it a correct assertion?

Yes. I am OK to either open another ticket for the upgrade, or another for the 32bit issue, but in any case, please do not just close that one without opening another ticket, since the issue is not solved (or only partially).

dimpase commented 6 years ago
comment:18

This upgrade is an improvement. Surely the 32-bit issue is not fully solved (the new bug is in new code as far as I can see), but this should go to a followup ticket.

fredrik-johansson commented 6 years ago
comment:19

Yes, the 32-bit bug is almost certainly in new code and should not be a concern for Sage.

dimpase commented 6 years ago
comment:20

The follow-up ticket is here: #24661

Let us finally be done with this one.

mezzarobba commented 6 years ago

Changed reviewer from Thierry Monteil to Thierry Monteil, Marc Mezzarobba, Fredrik Johansson

mezzarobba commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,5 +1,5 @@
-arb fails to pass self-tests on my 32bit VM, while it did not before. Log attached. As well, it fails to pass self-tests on x86_64 skylake.
+arb fails to pass self-tests on x86_64 skylake. This is fixed in arb 2.12.0.

-The latter is fixed in arb 2.12.0.
+(The new version fails to pass self-tests on my 32bit VM, while it did not before, see #24661.)

 tarball here: https://github.com/fredrik-johansson/arb/archive/2.12.0.tar.gz
vbraun commented 6 years ago

Changed branch from u/dimpase/arb212 to c793138

Konrad127123 commented 6 years ago
comment:23

This ticket has been closed, but as far as I can see, it's not been merged into the develop branch. Is that intentional?

Apologies if I'm missing something obvious.

(Edit: I don't know why trac says I deleted the commit.)

Konrad127123 commented 6 years ago

Changed commit from c793138 to none

dimpase commented 6 years ago
comment:24

Replying to @Konrad127123:

This ticket has been closed, but as far as I can see, it's not been merged into the develop branch. Is that intentional?

Apologies if I'm missing something obvious.

I think it will be in the next beta (it has been merged in a sort of trunk, and this happened later than the latest ticket in the current beta(5))

(Edit: I don't know why trac says I deleted the commit.)

this is just a quirk of the interface. In fact the branch u/dimpase/arb212 is still there if you need it, e.g. from the mirror:

https://github.com/sagemath/sagetrac-mirror/tree/u/dimpase/arb212

edd8e884-f507-429a-b577-5d554626c0fe commented 5 years ago

Changed keywords from none to sdl