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

Add abs tol to some CBF and RBF doctests #32905

Closed
slel opened this issue Nov 19, 2021 · 30 comments
Closed

Add abs tol to some CBF and RBF doctests #32905

slel opened this issue Nov 19, 2021 · 30 comments

Comments

@slel
Copy link
Member

slel commented Nov 19, 2021

Across arb versions, CBF and RBF can give
slightly different numerical results.

On an early 2014 MacBook Air, macOS 10.14.6,
many Homebrew packages including arb 2.21.1,
testing Sage 9.5.beta7 with random seed 0
gives failures in

  • src/sage/functions/gamma.py -- 1 doctest failed
  • src/sage/rings/complex_arb.pyx -- 3 doctests failed
  • src/sage/rings/real_arb.pyx -- 12 doctests failed
  • src/sage/symbolic/function.pyx -- 1 doctest failed

Following two related tickets merged in Sage 9.5.beta7,

here we adapt a few doctests to numerical variations
between arb 2.19.0 and arb 2.21.1.

CC: @dimpase @mezzarobba @slel @antonio-rojas

Component: numerical

Keywords: arb, CBF, RBF

Author: Samuel Lelièvre

Branch/Commit: 2ee7b99

Reviewer: Gonzalo Tornaría

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

@slel slel added this to the sage-9.5 milestone Nov 19, 2021
@slel
Copy link
Member Author

slel commented Nov 19, 2021

Author: Samuel Lelièvre

@slel
Copy link
Member Author

slel commented Nov 19, 2021

New commits:

b6d3bb5Adapt CBF and RBF doctests to numerical noise

@slel
Copy link
Member Author

slel commented Nov 19, 2021

Commit: b6d3bb5

@slel
Copy link
Member Author

slel commented Nov 19, 2021

Branch: u/slelievre/32905

@slel
Copy link
Member Author

slel commented Nov 19, 2021

comment:2

[Oops. Initially only considered two out of four files. Edited.]

With Sage 9.5.beta7:

$ ./sage -t --long --random-seed=0 \
  src/sage/functions/gamma.py \
  src/sage/rings/complex_arb.pyx \
  src/sage/rings/real_arb.pyx \
  src/sage/symbolic/function.pyx
too many failed tests, not using stored timings
Running doctests with ID 2021-11-19-17-19-26-ea3c3ab9.
Git branch: develop
Using --optional=build,dochtml,homebrew,pip,sage,sage.geometry.polyhedron,sage.rings.real_double,sage_spkg
Doctesting 4 files.
sage -t --long --random-seed=0 src/sage/functions/gamma.py
**********************************************************************
File "src/sage/functions/gamma.py", line 1052, in sage.functions.gamma.Function_beta._method_arguments
Failed example:
    RBF(beta(sin(3),sqrt(RBF(2).add_error(1e-8)/3)))
Expected:
    [7.407662 +/- 6.17e-7]
Got:
    [7.4076616 +/- 5.44e-8]
**********************************************************************
1 item had failures:
   1 of   2 in sage.functions.gamma.Function_beta._method_arguments
    [214 tests, 1 failure, 4.05 s]
sage -t --long --random-seed=0 src/sage/rings/complex_arb.pyx
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4192, in sage.rings.complex_arb.ComplexBall.Ei
Failed example:
    CBF(Ei(I))
Expected:
    [0.337403922900968 +/- 3.76e-16] + [2.51687939716208 +/- 2.01e-15]*I
Got:
    [0.337403922900968 +/- 4.31e-16] + [2.51687939716208 +/- 2.01e-15]*I
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4242, in sage.rings.complex_arb.ComplexBall.Ci
Failed example:
    CBF(Ci(I))
Expected:
    [0.837866940980208 +/- 4.72e-16] + [1.570796326794897 +/- 5.54e-16]*I
Got:
    [0.837866940980208 +/- 4.78e-16] + [1.570796326794897 +/- 5.54e-16]*I
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 4294, in sage.rings.complex_arb.ComplexBall.Chi
Failed example:
    CBF(Chi(I))
Expected:
    [0.337403922900968 +/- 3.25e-16] + [1.570796326794897 +/- 5.54e-16]*I
Got:
    [0.337403922900968 +/- 3.80e-16] + [1.570796326794897 +/- 5.54e-16]*I
**********************************************************************
3 items had failures:
   1 of   4 in sage.rings.complex_arb.ComplexBall.Chi
   1 of   4 in sage.rings.complex_arb.ComplexBall.Ci
   1 of   4 in sage.rings.complex_arb.ComplexBall.Ei
    [655 tests, 3 failures, 5.94 s]
sage -t --long --random-seed=0 src/sage/rings/real_arb.pyx
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3492, in sage.rings.real_arb.RealBall.Ei
Failed example:
    RBF(1).Ei()
Expected:
    [1.89511781635594 +/- 4.94e-15]
Got:
    [1.89511781635594 +/- 5.11e-15]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3497, in sage.rings.real_arb.RealBall.Ei
Failed example:
    RBF(Ei(1))
Expected:
    [1.89511781635594 +/- 4.94e-15]
Got:
    [1.89511781635594 +/- 5.11e-15]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3534, in sage.rings.real_arb.RealBall.Ci
Failed example:
    RBF(1).Ci()
Expected:
    [0.337403922900968 +/- 3.25e-16]
Got:
    [0.337403922900968 +/- 3.80e-16]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3539, in sage.rings.real_arb.RealBall.Ci
Failed example:
    RBF(Ci(1))
Expected:
    [0.337403922900968 +/- 3.25e-16]
Got:
    [0.337403922900968 +/- 3.80e-16]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3578, in sage.rings.real_arb.RealBall.Chi
Failed example:
    RBF(1).Chi()
Expected:
    [0.837866940980208 +/- 4.72e-16]
Got:
    [0.837866940980208 +/- 4.78e-16]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3583, in sage.rings.real_arb.RealBall.Chi
Failed example:
    RBF(Chi(1))
Expected:
    [0.837866940980208 +/- 4.72e-16]
Got:
    [0.837866940980208 +/- 4.78e-16]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3600, in sage.rings.real_arb.RealBall.li
Failed example:
    RBF(3).li()
Expected:
    [2.16358859466719 +/- 4.72e-15]
Got:
    [2.16358859466719 +/- 5.11e-15]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3624, in sage.rings.real_arb.RealBall.Li
Failed example:
    RBF(3).Li()
Expected:
    [1.11842481454970 +/- 7.61e-15]
Got:
    [1.11842481454970 +/- 8.00e-15]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3651, in sage.rings.real_arb.RealBall.beta
Failed example:
    RBF(sin(3)).beta(RBF(2/3).sqrt())
Expected:
    [7.407661629415 +/- 1.07e-13]
Got:
    [7.4076616294151 +/- 5.60e-14]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3653, in sage.rings.real_arb.RealBall.beta
Failed example:
    RealBallField(100)(7/2).beta(1)
Expected:
    [0.28571428571428571428571428571 +/- 5.23e-30]
Got:
    [0.28571428571428571428571428571 +/- 4.93e-30]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3688, in sage.rings.real_arb.RealBall.gamma
Failed example:
    RBF(gamma(3/2, RBF(2).sqrt()))
Expected:
    [0.37118875695353 +/- 3.00e-15]
Got:
    [0.37118875695353 +/- 3.01e-15]
**********************************************************************
File "src/sage/rings/real_arb.pyx", line 3690, in sage.rings.real_arb.RealBall.gamma
Failed example:
    RBF(3/2).gamma_inc(RBF(2).sqrt())
Expected:
    [0.37118875695353 +/- 3.00e-15]
Got:
    [0.37118875695353 +/- 3.01e-15]
**********************************************************************
7 items had failures:
   2 of   3 in sage.rings.real_arb.RealBall.Chi
   2 of   3 in sage.rings.real_arb.RealBall.Ci
   2 of   3 in sage.rings.real_arb.RealBall.Ei
   1 of   2 in sage.rings.real_arb.RealBall.Li
   2 of   4 in sage.rings.real_arb.RealBall.beta
   2 of   5 in sage.rings.real_arb.RealBall.gamma
   1 of   4 in sage.rings.real_arb.RealBall.li
    [568 tests, 12 failures, 0.61 s]
sage -t --long --random-seed=0 src/sage/symbolic/function.pyx
**********************************************************************
File "src/sage/symbolic/function.pyx", line 629, in sage.symbolic.function.Function._is_numerical
Failed example:
    gamma(b, 1)
Expected:
    [0.50728223 +/- 4.67e-9]
Got:
    [0.507282234 +/- 3.81e-10]
**********************************************************************
1 item had failures:
   1 of  12 in sage.symbolic.function.Function._is_numerical
    [245 tests, 1 failure, 2.70 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/functions/gamma.py  # 1 doctest failed
sage -t --long --random-seed=0 src/sage/rings/complex_arb.pyx  # 3 doctests failed
sage -t --long --random-seed=0 src/sage/rings/real_arb.pyx  # 12 doctests failed
sage -t --long --random-seed=0 src/sage/symbolic/function.pyx  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 14.0 seconds
    cpu time: 12.2 seconds
    cumulative wall time: 13.3 seconds
Pytest is not installed, skip checking tests that rely on it.

With the proposed branch (commit 859b7ba):

$ ./sage -t --long --random-seed=0 \
  src/sage/functions/gamma.py \
  src/sage/rings/complex_arb.pyx \
  src/sage/rings/real_arb.pyx \
  src/sage/symbolic/function.pyx
too many failed tests, not using stored timings
Running doctests with ID 2021-11-19-17-21-44-77772dac.
Git branch: t-32905
Using --optional=build,dochtml,homebrew,pip,sage,sage.geometry.polyhedron,sage.rings.real_double,sage_spkg
Doctesting 4 files.
sage -t --long --random-seed=0 src/sage/functions/gamma.py
    [214 tests, 4.63 s]
sage -t --long --random-seed=0 src/sage/rings/complex_arb.pyx
    [655 tests, 6.78 s]
sage -t --long --random-seed=0 src/sage/rings/real_arb.pyx
    [568 tests, 0.58 s]
sage -t --long --random-seed=0 src/sage/symbolic/function.pyx
    [245 tests, 2.09 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 14.8 seconds
    cpu time: 12.1 seconds
    cumulative wall time: 14.1 seconds
Pytest is not installed, skip checking tests that rely on it.

@slel
Copy link
Member Author

slel commented Nov 19, 2021

Changed commit from b6d3bb5 to 859b7ba

@slel
Copy link
Member Author

slel commented Nov 19, 2021

New commits:

859b7baAdapt CBF and RBF doctests to numerical noise

@slel
Copy link
Member Author

slel commented Nov 19, 2021

Changed branch from u/slelievre/32905 to public/32905

@slel

This comment has been minimized.

@mezzarobba
Copy link
Member

comment:4

(Not a review, just some comments.)

The results should only depend on the installed version of arb, not on other system details. The policy until now (at Jeroen's insistence long ago, if I remember correctly) has been to doctest the exact output corresponding to the version packaged by sage-the-distribution. With increasing use of system packages, it may be time to change that, but that should be a conscious decision.

I don't see the point of things like [3.141592653589793 +/- ...e-16]: if all we are interested in is that the value (and its approximate accuracy), we may as well write [3.14159265358979...]; if, on the other hand, we want (for documentation purposes, say) the shape of the output to be apparent, then something like [3.141592653589793 +/- 3.39e-16] # abs tol 1e-15 seems better.

@slel

This comment has been minimized.

@slel
Copy link
Member Author

slel commented Nov 19, 2021

comment:5

Thanks for clarifying that results only vary
with arb versions, not other system details.
I edited the ticket description accordingly.

The policy you mention does not seem to have
been consistently enforced, as many doctests
near those I changed have ... ellipses in
the output.

That said, your suggestions make a lot of sense.

I prefer using abs tol to an ellipsis as
it better shows the shape of the output and
better controls the doctest output (the ellipsis
would match a wrong imaginary part).

@dimpase
Copy link
Member

dimpase commented Nov 19, 2021

comment:6

For the same package versions, answers might be a bit different on different hardware, in particular if machine floating point is involved in any way.

But yes, #abs tol is the way to go. Does it work with complex data?

@slel
Copy link
Member Author

slel commented Nov 19, 2021

comment:7

My understanding is abs tol or rel tol
apply to each floating-point number in the output.

So for instance in the following output:

sage: CBF(pi + I*e)  # abs tol 3e-16
[3.141592653589793 +/- 3.39e-16] + [2.718281828459045 +/- 5.41e-16]*I

the doctest would test that we have something of the form

[AAA +/- BBB] + [CCC +/- DDD]*I

where each of AAA, BBB, CCC, DDD is
a floating-point number, and that they match,
each within the given tolerance, the values
3.141592653589793, 3.39e-16,
2.718281828459045, 5.41e-16.

@slel
Copy link
Member Author

slel commented Nov 19, 2021

comment:8

Did doctests from #32869 and #32851 use
the exact output obtained using Arb 2.19?

In comment:2 above, the tests were run using
Homebrew's "arb 2.21.1".

To adapt CBF and RBF doctests with abs tol,
I could compare the outputs given by

  • Sage with Homebrew's Arb 2.21.1
$ brew list --versions arb
arb 2.21.1
  • Sage with the arb spkg, currently at:
$ cat build/pkgs/arb/package-version.txt
2.19.0.p0

But beyond that I don't know the range of
allowed arb versions or how to figure it out.

Checking the outputs of each CBF and RBF doctest
across our tox configurations (which might reflect
the range of allowed arb versions to some extent)
seems a bit daunting.

One way might be to use the exact output
from Sage-with-arb-spkg, add some tolerance,
run some github-actions to check that worked,
and iterate if not. I wonder how those
github-actions could be made to start from
an already built Sage 9.5.beta7 and to test
only the few files we care about here.

Related: ticket #32211 will upgrade
FLINT and Arb.

@mezzarobba
Copy link
Member

comment:9

Replying to @dimpase:

For the same package versions, answers might be a bit different on different hardware, in particular if machine floating point is involved in any way.

In principle yes, but I am not aware of any example of this situation with arb. (This is frequent with libm operations, less so with basic IEEE-754 correctly rounded operations, and I suspect small variations in the computation steps for which arb uses hardware FP typically don't affect the final result at all.)

@mezzarobba
Copy link
Member

comment:10

Replying to @slel:

Did doctests from #32869 and #32851 use
the exact output obtained using Arb 2.19?

Yes.

But beyond that I don't know the range of
allowed arb versions or how to figure it out.

spkg-configure checks for arb ≥2.16; I don't know if that's really enough or if we now use more recent additions somewhere.

I don't think the precise versions matter much for most doctests: typically all that happens with new versions is that some error bounds change a little bit but remain of the same order of magnitude.

@slel
Copy link
Member Author

slel commented Nov 20, 2021

comment:11

On the same machine as in comment:2,
another copy of Sage built with the arb spkg:

$ source .homebrew-build-env
$ ./bootstrap -q
$ ./configure --with-system-arb=no -q
$ make -s V=0 ptestlong

passes all tests in the four affected files
for unmodified Sage 9.5.beta7.

I'll prepare a new branch with tolerance indications.

@slel
Copy link
Member Author

slel commented Nov 20, 2021

Changed commit from 859b7ba to e719be1

@slel
Copy link
Member Author

slel commented Nov 20, 2021

comment:13

Here is enough to let all CBF and RBF doctests pass,
whether using arb 2.21.0 or arb 2.19.0.

Replacing ... by abs tol in all CBF and RBF
doctests can be a different ticket.

Getting the minimal fix from this branch
into Sage 9.5 is more urgent in my eyes.


New commits:

e719be132905: abs tol for some CBF and RBF doctests

@slel
Copy link
Member Author

slel commented Nov 20, 2021

Changed branch from public/32905 to u/slelievre/ticket/32905

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 20, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2ee7b9932905: abs tol for some CBF and RBF doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 20, 2021

Changed commit from e719be1 to 2ee7b99

@slel
Copy link
Member Author

slel commented Nov 20, 2021

comment:15

I had left out the changes to real_arb.pyx,
I force-pushed to include them. Ready for review now.

@slel

This comment has been minimized.

@slel slel changed the title Adapt CBF and RBF doctests to numerical noise Add abs tol to some CBF and RBF doctests Nov 20, 2021
@tornaria
Copy link
Contributor

comment:17

Looks good to me. I took the patch for the sagemath package for void linux (wip) which uses arb 2.21.1, and all of those tests that were failing now pass.

See void-linux/void-packages#34030

@tornaria
Copy link
Contributor

Reviewer: Gonzalo Tornaría

@antonio-rojas
Copy link
Contributor

comment:18

Raising priority to put it in the 9.5 radar, as this is affecting many distro packages.

@slel
Copy link
Member Author

slel commented Dec 17, 2021

@vbraun
Copy link
Member

vbraun commented Dec 19, 2021

Changed branch from u/slelievre/ticket/32905 to 2ee7b99

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

6 participants