Opened 4 years ago

Closed 2 years ago

#23430 closed enhancement (fixed)

Cannot create a RealBall with rational radius

Reported by: deinst Owned by:
Priority: minor Milestone: sage-8.4
Component: numerical Keywords: sagedays@icerm
Cc: tmonteil, mmezzarobba, vdelecroix, ​fredrik.johansson Merged in:
Authors: David Einstein Reviewers: Bryan Gin-ge Chen
Report Upstream: N/A Work issues:
Branch: 457c68d (Commits) Commit: 457c68d34a06b2dcdfe32ebb5757aeacc2727407
Dependencies: Stopgaps:

Description

For example

sage: RBF(1, 1/2)

will fail with unable to convert 1 to a RealBall but

sage: RBF(1, 0.5)

will succeed. If this is not intentional I'll put a patch together.

Change History (14)

comment:1 Changed 3 years ago by mmezzarobba

Well, I intentionally left that out at first when writing the bulk of the constructor code, and then never got around to implementing it, but I certainly agree that it would be good to have. So feel free to go ahead and add it!

Last edited 3 years ago by mmezzarobba (previous) (diff)

comment:2 Changed 2 years ago by deinst

  • Branch set to u/deinst/cannot_create_a_realball_with_rational_radius

comment:3 Changed 2 years ago by deinst

  • Commit set to a1123e63caa4be480f624d57cb01358cedfa955e
  • Keywords sagedays@icerm added
  • Priority changed from major to minor

comment:4 Changed 2 years ago by git

  • Commit changed from a1123e63caa4be480f624d57cb01358cedfa955e to bfab4421ab5b46767b03b659212ae870d79370c1

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

bfab442Used the proper precision.

comment:5 Changed 2 years ago by deinst

  • Status changed from new to needs_review

comment:6 Changed 2 years ago by gh-bryangingechen

It looks like the line cdef Integer foo should be removed.

This test:

sage: RBF(13, 1)
[1e+1 +/- 4.01]

looks crazy to me. Namely, why is the radius 1 printed as 4.01? I get the same output in the current version of Sage if I put in RBF(13, 1.), so this behavior has been around for a while. Is it actually correct / expected? What am I missing?

comment:7 Changed 2 years ago by git

  • Commit changed from bfab4421ab5b46767b03b659212ae870d79370c1 to 457c68d34a06b2dcdfe32ebb5757aeacc2727407

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

457c68dRemoved spurious declaration.

comment:8 Changed 2 years ago by tscrim

  • Cc tmonteil mmezzarobba vdelecroix ​fredrik.johansson added
  • Milestone changed from sage-8.1 to sage-8.4

@gh-bryangingechen My guess is that it is an error with the printing (maybe with upstream):

sage: RBF(13,0.89)
[1.3e+1 +/- 0.891]
sage: RBF(11,0.9)
[1e+1 +/- 1.91]
sage: RBF(12,0.9)
[1e+1 +/- 2.91]
sage: RBF(13,0.9)
[1e+1 +/- 3.91]
sage: RBF(14,0.9)
[1e+1 +/- 4.91]
sage: RBF(15,0.9)
[2e+1 +/- 5.91]
sage: x = RBF(14,0.9)
sage: x.center()
14.0000000000000
sage: x.rad()
0.90000000
sage: x.rad().parent()
Real Field with 30 bits of precision
sage: (0.9).parent()
Real Field with 53 bits of precision
sage: x.center().parent()
Real Field with 53 bits of precision

Should there be a precision loss with the radius too?

David, don't forget to add your real name to the authors field.

I think this also might fix #25456.

comment:9 Changed 2 years ago by deinst

  • Authors set to David Einstein

Sorry about the extra foo.

The printing is a consequence of Arb's printing. The arb function arb_get_str returns a string that is guaranteed to contain the original ball when read back in and is rounded so that it is accurate to one unit in the last decimal place. This behavior can be puzzling particularly when the error magnitude is not much smaller than the absolute value of of the midpoint.

comment:10 Changed 2 years ago by gh-bryangingechen

  • Reviewers set to Bryan Gin-ge Chen
  • Status changed from needs_review to positive_review

OK, as long as the print behavior is known. It might be good to open a ticket to document it a bit more, though perhaps #15944 will eventually cover it.

comment:11 Changed 2 years ago by vdelecroix

  • Status changed from positive_review to needs_info

Have you looked at #25456? Have you read https://trac.sagemath.org/ticket/25456#comment:4? There is a good reason for not doing what you propose.

comment:12 Changed 2 years ago by vdelecroix

  • Status changed from needs_info to positive_review

Though, here it only deals with integer/rationals.

comment:13 Changed 2 years ago by deinst

I have looked at #25456 but I have been, I think, careful to ensure that the arb ball includes the rational number in question.

comment:14 Changed 2 years ago by vbraun

  • Branch changed from u/deinst/cannot_create_a_realball_with_rational_radius to 457c68d34a06b2dcdfe32ebb5757aeacc2727407
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.