Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#25966 closed enhancement (fixed)

Upgrade Arb to 2.14.0

Reported by: fredrik.johansson Owned by:
Priority: major Milestone: sage-8.4
Component: packages: standard Keywords:
Cc: gh-timokau, jdemeyer Merged in:
Authors: Fredrik Johansson Reviewers: Travis Scrimshaw, Timo Kaufmann
Report Upstream: N/A Work issues:
Branch: 8bef4fd (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by fredrik.johansson)

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

Because of more accurate error bounds (mainly for trigonometric functions), a large number of doctests have to be updated.

Change History (16)

comment:1 Changed 3 years ago by fredrik.johansson

  • Branch set to u/fredrik.johansson/upgrade_arb_to_2_14_0

comment:2 Changed 3 years ago by fredrik.johansson

  • Commit set to d4295f0ca10661a5fc97d7656d844e9deba74c84
  • Component changed from PLEASE CHANGE to packages: standard
  • Description modified (diff)
  • Status changed from new to needs_review
  • Type changed from PLEASE CHANGE to enhancement

New commits:

d22edeeupdate arb to 2.14.0
6f0646bupdate doctests affected by accuracy changes in arb 2.14
d4295f0Merge branch 'arb214' into t/25966/upgrade_arb_to_2_14_0

comment:3 Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Buildbots ahoy! *cough*authorname*cough*

comment:4 Changed 3 years ago by tscrim

  • Authors set to Fredrik Johansson

comment:5 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work
**********************************************************************
File "src/sage/rings/complex_arb.pyx", line 1029, in sage.rings.complex_arb.ComplexBallField.?
Failed example:
    C.integral(lambda x, _: (x + x.exp()).sin(), 0, 8) # long time
Expected:
    [0.34740017...55347713 +/- 6.72e-598]
Got:
    [0.347400172657247807879512159119893124657456254866180183885492713616748213988785320529685104346604105756813796172006018707302714228197618073704004353678452866627436279471970216410908716043577412909956068587776704710948691279593004567821091508925369957240639547298886452332685264381903039098870525160076005978168808062746564134987731050142119306346307812114504420398584159417131096900798039662018221641127266271301569159908541386062975763888813212164702872661085356464323603726722886936732008463756755254875678750234857309302489895429562039632272592695502596880447563060128384223515181416786355347713 +/- 5.71e-598]
**********************************************************************
1 item had failures:
   1 of  56 in sage.rings.complex_arb.ComplexBallField.?
    [622 tests, 1 failure, 3.69 s]
----------------------------------------------------------------------
sage -t --long src/sage/rings/complex_arb.pyx  # 1 doctest failed
----------------------------------------------------------------------

comment:6 Changed 3 years ago by gh-timokau

  • Cc gh-timokau added

comment:7 Changed 3 years ago by tscrim

  • Branch changed from u/fredrik.johansson/upgrade_arb_to_2_14_0 to u/tscrim/upgrade_arb_to_2_14_0
  • Commit changed from d4295f0ca10661a5fc97d7656d844e9deba74c84 to 8bef4fd2876a61969b516fe4eb3b8ad7cc076c5e
  • Status changed from needs_work to needs_review

I must have done the same thing and not run --long. Mea culpa. Fixed.


New commits:

61e2d8eMerge branch 'u/fredrik.johansson/upgrade_arb_to_2_14_0' of git://trac.sagemath.org/sage into u/fredrik.johansson/upgrade_arb_to_2_14_0
8bef4fdFix one doctest.

comment:8 follow-up: Changed 3 years ago by fredrik.johansson

Am I allowed to give the positive review to the reviewer's patch? Are we waiting for other reviewing?

comment:9 in reply to: ↑ 8 Changed 3 years ago by tscrim

Replying to fredrik.johansson:

Am I allowed to give the positive review to the reviewer's patch?

Yes.

Are we waiting for other reviewing?

Not that I am aware of.

comment:10 Changed 3 years ago by fredrik.johansson

Unfortunately Sage fails to build for me right now (for unrelated reasons) so I can't verify that the tests pass.

comment:11 Changed 3 years ago by gh-timokau

  • Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Timo Kaufmann
  • Status changed from needs_review to positive_review

Tests pass.

comment:12 follow-up: Changed 3 years ago by gh-timokau

In the future it would be preferable to make use of # abs tol 1e-42 feature to reduce the doctest precision. It would make the doctests less brittle.

comment:13 Changed 3 years ago by vbraun

  • Branch changed from u/tscrim/upgrade_arb_to_2_14_0 to 8bef4fd2876a61969b516fe4eb3b8ad7cc076c5e
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 in reply to: ↑ 12 Changed 3 years ago by mmezzarobba

  • Cc jdemeyer added
  • Commit 8bef4fd2876a61969b516fe4eb3b8ad7cc076c5e deleted

Replying to gh-timokau:

In the future it would be preferable to make use of # abs tol 1e-42 feature to reduce the doctest precision. It would make the doctests less brittle.

See the discussion at #18560. IMO it would be time to switch to using tolerances indeed. Jeroen, did you view of the issue change since that ticket?

comment:15 follow-up: Changed 3 years ago by fredrik.johansson

Differences when upgrading Arb are not a big issue IMO. In fact, it's helpful (for me) to look at the Sage doctests to see what improved or got worse between two versions. However, divergence between 32-bit and 64-bit results is plausible in the future and tolerances may be the best solution for that.

comment:16 in reply to: ↑ 15 Changed 3 years ago by gh-timokau

Replying to fredrik.johansson:

Differences when upgrading Arb are not a big issue IMO. In fact, it's helpful (for me) to look at the Sage doctests to see what improved or got worse between two versions. However, divergence between 32-bit and 64-bit results is plausible in the future and tolerances may be the best solution for that.

They are an issue when using sage outside of sage-the-distribution. They make it necessary that either the arb version used is the exact arb version sage-the-distribution uses or that patches are applied to sage. With tolerances we could potentially update sage and arb independently. And this is not only an issue for distributions. It could in the long run also allow sage-the-distribution to use the system's arb version (#24919).

In my opinion ideal sage tests should only fail if something is actually broken in sage, upstream projects or packaging. "broken" here may be an actual error or a api change.

Note: See TracTickets for help on using tickets.