#24927 closed enhancement (fixed)

Upgrade arb to 2.13.0

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.3
Component: packages: standard Keywords:
Cc: mmezzarobba, fredrik.johansson, fbissey Merged in:
Authors: Ralf Stephan, Frédéric Chapoton, Marc Mezzarobba Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: e0e3c06 (Commits) Commit: e0e3c06f561515d83b0c8803639f26efa4f5bf11
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

Upstream at

https://github.com/fredrik-johansson/arb/archive/2.13.0.tar.gz

(to be renamed arb-2.13.0.tar.gz)

Many doctests have to be changed because 1/0 now returns nan rather than [+/- inf]. See also the announcement on flint-devel.

Issues about enclosure regressions reported upstream at Issue #227.

Change History (37)

comment:1 Changed 22 months ago by fbissey

  • Cc fbissey added

comment:2 Changed 22 months ago by vdelecroix

  • Dependencies set to #24575

comment:3 Changed 22 months ago by vdelecroix

  • Description modified (diff)

comment:4 Changed 22 months ago by vdelecroix

Among the changed behavior some of them are weird (and perhaps not desirable)

**********************************************************************
File "complex_arb.pyx", line 2287, in sage.rings.complex_arb.ComplexBall.log
Failed example:
    CBF('inf').log()
Expected:
    nan + nan*I
Got:
    [+/- inf]
**********************************************************************
File "complex_arb.pyx", line 2395, in sage.rings.complex_arb.ComplexBall.tan
Failed example:
    CBF(pi/2).tan()
Expected:
    [+/- inf]
Got:
    nan
**********************************************************************
File "complex_arb.pyx", line 2412, in sage.rings.complex_arb.ComplexBall.cot
Failed example:
    CBF(pi).cot()
Expected:
    [+/- inf]
Got:
    nan
**********************************************************************
File "real_arb.pyx", line 3124, in sage.rings.real_arb.RealBall.tan
Failed example:
    RBF(pi/2).tan()
Expected:
    [+/- inf]
Got:
    nan
**********************************************************************
File "real_arb.pyx", line 3141, in sage.rings.real_arb.RealBall.cot
Failed example:
    RBF(pi).cot()
Expected:
    [+/- inf]
Got:
    nan
**********************************************************************
File "real_arb.pyx", line 3252, in sage.rings.real_arb.RealBall.coth
Failed example:
    RBF(0).coth()
Expected:
    [+/- inf]
Got:
    nan

I also noted some small regressions in enclosures (ignoring the ones that get better)

**********************************************************************
File "complex_arb.pyx", line 2798, in sage.rings.complex_arb.ComplexBall.hypergeometric
Failed example:
    CBF(2+3*I).hypergeometric([1/4,1/3],[1/2])
Expected:
    [0.7871684267473 +/- 7.34e-14] + [0.2749254173721 +/- 9.23e-14]*I
Got:
    [0.7871684267473 +/- 7.38e-14] + [0.2749254173721 +/- 9.22e-14]*I
**********************************************************************
File "complex_arb.pyx", line 2800, in sage.rings.complex_arb.ComplexBall.hypergeometric
Failed example:
    CBF(2+3*I).hypergeometric([1/4,1/3],[1/2],regularized=True)
Expected:
    [0.4441122268685 +/- 3.96e-14] + [0.1551100567338 +/- 5.75e-14]*I
Got:
    [0.4441122268685 +/- 3.98e-14] + [0.1551100567338 +/- 5.77e-14]*I
**********************************************************************
File "complex_arb.pyx", line 3648, in sage.rings.complex_arb.ComplexBall.legendre_P
Failed example:
    CBF(1/2).legendre_P(5)
Expected:
    [0.08984375000000000 +/- 4.5...e-18]
Got:
    [0.0898437500000000 +/- 7.29e-17]
**********************************************************************
File "complex_arb.pyx", line 3686, in sage.rings.complex_arb.ComplexBall.legendre_Q
Failed example:
    CBF(-10).legendre_Q(5, 325/100)
Expected:
    [-83825154.36008 +/- 4.94e-6] + [-34721515.80396 +/- 5.40e-6]*I
Got:
    [-83825154.36008 +/- 5.02e-6] + [-34721515.80396 +/- 5.42e-6]*I
Last edited 20 months ago by vdelecroix (previous) (diff)

comment:5 follow-up: Changed 22 months ago by mmezzarobba

It may be better to wait for the existing arb-related tickets to be merged, in case the upgrade causes doctest failures there...

comment:6 follow-up: Changed 22 months ago by vdelecroix

And this is annoying

sage: RBF([-infinity,infinity])
nan

comment:7 in reply to: ↑ 5 ; follow-up: Changed 22 months ago by vdelecroix

Replying to mmezzarobba:

It may be better to wait for the existing arb-related tickets to be merged, in case the upgrade causes doctest failures there...

Could you add them as dependencies?

Last edited 22 months ago by vdelecroix (previous) (diff)

comment:8 in reply to: ↑ 7 Changed 22 months ago by mmezzarobba

Replying to vdelecroix:

Replying to mmezzarobba:

It may be better to wait for the existing arb-related tickets to be merged, in case the upgrade causes doctest failures there...

Could you add them as dependencies?

Yes, but I also wouldn't want to block this ticket if nobody is interested in reviewing the other ones. In any case, the tickets I have in mind are #24314, #24816, #24717, #24626.

comment:9 Changed 22 months ago by vdelecroix

  • Dependencies changed from #24575 to #24575, #24314, #24816, #24717, #24626

comment:10 Changed 20 months ago by rws

Coming late to the party, pynac-0.7.17 (#24838) depends on this because a fix is included.

comment:11 follow-up: Changed 20 months ago by jdemeyer

Why is #24575 a dependency? That seems to have nothing to do with arb.

comment:12 in reply to: ↑ 11 Changed 20 months ago by vdelecroix

  • Dependencies changed from #24575, #24314, #24816, #24717, #24626 to #24314, #24816, #24717, #24626

Replying to jdemeyer:

Why is #24575 a dependency? That seems to have nothing to do with arb.

Somehow it does. The Makefile of arb is making use of LD_LIBRARY_PATH and #24575 aimed to change that...

comment:13 Changed 20 months ago by vdelecroix

  • Description modified (diff)
  • Milestone changed from sage-8.2 to sage-8.3

comment:14 Changed 19 months ago by rws

Dependencies are merged now. Anyone working on this?

comment:15 Changed 19 months ago by rws

  • Branch set to u/rws/upgrade_arb_to_2_13_0

comment:16 Changed 19 months ago by rws

  • Commit set to 9c8b1c7250b7a9cd2cca0660d3b5ee38f53bad89
sage -t --warn-long 52.8 src/sage/rings/complex_arb.pyx  # 35 doctests failed
sage -t --warn-long 52.8 src/sage/rings/real_arb.pyx  # 10 doctests failed
sage -t --warn-long 52.8 src/sage/rings/polynomial/polynomial_complex_arb.pyx  # 2 doctests failed

Please someone go ahead. I can't work on this until end of June.


New commits:

9c8b1c724927: version/chksum/rm patch

comment:17 Changed 18 months ago by chapoton

  • Branch changed from u/rws/upgrade_arb_to_2_13_0 to public/24927
  • Commit changed from 9c8b1c7250b7a9cd2cca0660d3b5ee38f53bad89 to 0845bcdc6125ea14d1fc05e6e21a2a2b02949f63
  • Dependencies #24314, #24816, #24717, #24626 deleted

New commits:

f7c53b6Merge branch 'u/rws/upgrade_arb_to_2_13_0' in 8.3.b5
0845bcdtrying to fix arb doctests

comment:18 Changed 18 months ago by git

  • Commit changed from 0845bcdc6125ea14d1fc05e6e21a2a2b02949f63 to 50ac79a14624a50103379478e9fcfb214710596f

Branch pushed to git repo; I updated commit sha1. New commits:

50ac79atry to fix arb CBF doctests

comment:19 follow-up: Changed 18 months ago by chapoton

One of the remaining doctest failures come from the following.

There is a regression, before:

sage: sin(1/RBF(0))*0
0

after:

sage: sin(1/RBF(0))*0
nan

comment:20 in reply to: ↑ 19 Changed 18 months ago by rws

Replying to chapoton:

sage: sin(1/RBF(0))*0
nan

That's because of

sage: 1/RBF(0)
nan

which was infinite before.

comment:21 follow-ups: Changed 18 months ago by chapoton

Indeed. But what is the semantic of "nan" ? Should we expect sin(nan) to be [+/- 1] ?

comment:22 in reply to: ↑ 21 Changed 18 months ago by mmezzarobba

Replying to chapoton:

Indeed. But what is the semantic of "nan" ?

http://arblib.org/arb.html

Should we expect sin(nan) to be [+/- 1] ?

No. And I think that's more or less the point of the change: sin(1/x) ∈ [-1,1] for all real x close to zero, but returning a finite ball could give the impression that sin(1/x) is analytic at zero (since, for complex balls, finite result ⇒ analytic).

In any case, the resulting enclosure is correct (if coarser than before), and if upstream decided to make that change, I think we should follow along.

comment:23 Changed 18 months ago by git

  • Commit changed from 50ac79a14624a50103379478e9fcfb214710596f to 2662caf487050118cd300fb6b8c5c17453bc0e36

Branch pushed to git repo; I updated commit sha1. New commits:

2662caffixing doctests of oscillating integral of x sin(1/x)

comment:24 Changed 18 months ago by git

  • Commit changed from 2662caf487050118cd300fb6b8c5c17453bc0e36 to ba31bd51a1338dc2938a8d60d95fb6d10a7acee3

Branch pushed to git repo; I updated commit sha1. New commits:

ba31bd5fix doctests in real_arb by removing extended_line

comment:25 Changed 18 months ago by chapoton

  • Status changed from new to needs_info

I may have made some controversial changes.. please tell me.

comment:26 in reply to: ↑ 21 Changed 18 months ago by rws

Replying to chapoton:

Indeed. But what is the semantic of "nan" ? Should we expect sin(nan) to be [+/- 1] ?

FWIW, nan/NaN always leads to collapse of the expression tree.

comment:27 Changed 18 months ago by git

  • Commit changed from ba31bd51a1338dc2938a8d60d95fb6d10a7acee3 to 33186762d670825068722a44c6212002f98a9ccd

Branch pushed to git repo; I updated commit sha1. New commits:

8a65a15Revert "fix doctests in real_arb by removing extended_line"
3318676#24927 improve or fix some of Frédéric's doctest fixes

comment:28 Changed 18 months ago by git

  • Commit changed from 33186762d670825068722a44c6212002f98a9ccd to b29c09685f032ac2874f083f4fe70516464f5be7

Branch pushed to git repo; I updated commit sha1. New commits:

b29c096#24927 remove old workaround, add a new one...

comment:29 in reply to: ↑ 6 Changed 18 months ago by mmezzarobba

  • Authors set to Ralf Stephan, Frédéric Chapoton, Marc Mezzarobba
  • Status changed from needs_info to needs_review

Replying to vdelecroix:

And this is annoying

sage: RBF([-infinity,infinity])
nan

I don't think this is a new issue, but I took the occasion to fix it.

comment:30 Changed 18 months ago by rws

make ptestlong gives me two failures:

File "src/sage/functions/other.py", line 1418, in sage.functions.other.Function_factorial._eval_
Failed example:
    factorial(RBF(2)**64)
Expected:
    [+/- 2.30e+347382171305201370464]
Got:
    [+/- 2.30e+347382171326740403407]

File "src/sage/matrix/matrix_complex_ball_dense.pyx", line 637, in sage.matrix.matrix_complex_ball_dense.Matrix_complex_ball_dense.charpoly
Failed example:
    mat.charpoly(algorithm="hessenberg")
Expected:
    x^5 + ([-1.8 +/- 0.0445])*x^4 + ([0.3 +/- 0.0833])*x^3
    + ([+/- 0.0164])*x^2 + ([+/- 6.01e-4])*x + [+/- 6.97e-6]
Got:
    x^5 + ([-1.8 +/- 0.0445])*x^4 + ([0.3 +/- 0.0828])*x^3 + ([+/- 0.0163])*x^2 + ([+/- 5.95e-4])*x + [+/- 6.83e-6]

In the first one, using Stirling's formula confirms the regression:

sage: (log(sqrt(2*pi*2^64),10) + log(2^64/e,10)*2^64).n()
3.47382171305201e20

comment:31 follow-up: Changed 18 months ago by rws

  • Reviewers set to Ralf Stephan

If you mark the first test above "known bug", and fix the second then it's positive review from me.

comment:32 in reply to: ↑ 31 ; follow-up: Changed 18 months ago by mmezzarobba

Replying to rws:

If you mark the first test above "known bug", and fix the second then it's positive review from me.

It is not a bug, even if the enclosure is a bit less tight as it used to... I think we should just change the expected output.

comment:33 Changed 18 months ago by git

  • Commit changed from b29c09685f032ac2874f083f4fe70516464f5be7 to bebabb48df4c288e4874aac3f3aa6cec77265d08

Branch pushed to git repo; I updated commit sha1. New commits:

bebabb4trac 24927 fixing 2 doctests (one is tagged known bug)

comment:34 Changed 18 months ago by git

  • Commit changed from bebabb48df4c288e4874aac3f3aa6cec77265d08 to e0e3c06f561515d83b0c8803639f26efa4f5bf11

Branch pushed to git repo; I updated commit sha1. New commits:

e0e3c06change one doctest result

comment:35 in reply to: ↑ 32 Changed 18 months ago by chapoton

Replying to mmezzarobba:

Replying to rws:

If you mark the first test above "known bug", and fix the second then it's positive review from me.

It is not a bug, even if the enclosure is a bit less tight as it used to... I think we should just change the expected output.

ok, I just did that

comment:36 Changed 18 months ago by rws

  • Status changed from needs_review to positive_review

Thanks.

comment:37 Changed 18 months ago by vbraun

  • Branch changed from public/24927 to e0e3c06f561515d83b0c8803639f26efa4f5bf11
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.