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:  sage8.4 
Component:  numerical  Keywords:  sagedays@icerm 
Cc:  tmonteil, mmezzarobba, vdelecroix, fredrik.johansson  Merged in:  
Authors:  David Einstein  Reviewers:  Bryan Ginge 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
comment:2 Changed 2 years ago by
 Branch set to u/deinst/cannot_create_a_realball_with_rational_radius
comment:3 Changed 2 years ago by
 Commit set to a1123e63caa4be480f624d57cb01358cedfa955e
 Keywords sagedays@icerm added
 Priority changed from major to minor
comment:4 Changed 2 years ago by
 Commit changed from a1123e63caa4be480f624d57cb01358cedfa955e to bfab4421ab5b46767b03b659212ae870d79370c1
Branch pushed to git repo; I updated commit sha1. New commits:
bfab442  Used the proper precision.

comment:5 Changed 2 years ago by
 Status changed from new to needs_review
comment:6 Changed 2 years ago by
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
 Commit changed from bfab4421ab5b46767b03b659212ae870d79370c1 to 457c68d34a06b2dcdfe32ebb5757aeacc2727407
Branch pushed to git repo; I updated commit sha1. New commits:
457c68d  Removed spurious declaration.

comment:8 Changed 2 years ago by
 Cc tmonteil mmezzarobba vdelecroix fredrik.johansson added
 Milestone changed from sage8.1 to sage8.4
@ghbryangingechen 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
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
 Reviewers set to Bryan Ginge 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
 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
 Status changed from needs_info to positive_review
Though, here it only deals with integer/rationals.
comment:13 Changed 2 years ago by
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
 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
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!