Opened 9 years ago
Closed 5 years ago
#13055 closed enhancement (fixed)
Refactor numerical_approx()
Reported by:  dsm  Owned by:  burcin 

Priority:  minor  Milestone:  sage7.3 
Component:  basic arithmetic  Keywords:  numerical_approx, sd40.5, days74 
Cc:  benjaminjones  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Marc Mezzarobba 
Report Upstream:  N/A  Work issues:  
Branch:  fd79227 (Commits, GitHub, GitLab)  Commit:  fd79227f589f50f190066f7e636356007544b556 
Dependencies:  Stopgaps: 
Description (last modified by )
Drop the _numerical_approx
method, instead use numerical_approx
. Move the generic implementation to a new Cython file. Also put the conversion of decimal digits to bits in one place.
Finally, the documentation of numerical_approx
should be made more clear and consistent.
Change History (29)
comment:1 Changed 9 years ago by
 Description modified (diff)
comment:2 Changed 9 years ago by
 Keywords sd40.5 added
comment:3 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:4 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:5 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:6 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:7 Changed 7 years ago by
 Description modified (diff)
 Summary changed from loss of precision via SR to Better default precision for .n() method
comment:8 Changed 7 years ago by
 Description modified (diff)
comment:9 Changed 7 years ago by
 Type changed from defect to enhancement
comment:10 Changed 7 years ago by
 Milestone changed from sage6.4 to sageduplicate/invalid/wontfix
 Status changed from new to needs_review
comment:11 followup: ↓ 12 Changed 7 years ago by
Hmm, I'm not sure this is wontfix, though. I'm sympathetic to your argument, but Doug's point that
sage: sin(1.2345678901234567890) 0.9440057250452665781 sage: sin(1.2345678901234567890).n() 0.944005725045267
seems good; you shouldn't lose precision, should you? (Forget about hold!)
comment:12 in reply to: ↑ 11 Changed 7 years ago by
Losing precision is a feature of the .n()
method, that's what it does. The x.n()
method asks for a numerical approximation of x
to a precision of 53 bits. This is easily explained. I also like the fact that neither the implementation nor the user should need to worry about what x
is, the output is a real/complex number with 53 bits of precision.
If you think that .n()
is wrong, what should (2/3).n()
return then? The only possible answer which doesn't lose precision is the rational 2/3.
comment:13 followup: ↓ 14 Changed 7 years ago by
About hold=True
, that even doesn't seem to work with real numbers:
sage: gamma(1.2345678901234567890, hold=True) 0.9097200568693150729
comment:14 in reply to: ↑ 13 ; followup: ↓ 16 Changed 7 years ago by
 Milestone changed from sageduplicate/invalid/wontfix to sage6.5
 Priority changed from major to minor
 Status changed from needs_review to needs_work
 Summary changed from Better default precision for .n() method to Document default precision for .n() method
About
hold=True
, that even doesn't seem to work with real numbers:
Exactly, which is why I didn't use that example.
Okay, I don't have that much invested in this. But then
sage: x = 1.2340000000000000000000001 sage: x.n? Docstring: Return a numerical approximation of x with at least prec bits of precision. EXAMPLES: sage: (2/3).n() 0.666666666666667 sage: pi.n(digits=10) 3.141592654 sage: pi.n(prec=20) # 20 bits 3.1416 TESTS: Check that http://trac.sagemath.org/14778 is fixed: sage: (0).n(algorithm='foo') 0.000000000000000
should really be improved, and possibly the documentation for other versions of .numerical_approx
as well. Those are practically stubs!
comment:15 Changed 7 years ago by
 Component changed from symbolics to documentation
comment:16 in reply to: ↑ 14 Changed 7 years ago by
Replying to kcrisman:
Okay, I don't have that much invested in this. But then
x.n?
should really be improved, and possibly the documentation for other versions of.numerical_approx
as well. Those are practically stubs!
This is actually a general problem we have with Sage documentation: often one has similar functions/methods which all do the same thing (on different kinds of objects). In this case, the numerical_approx
function in src/sage/misc/functional.py
is much better documented. What's the optimal solution which doesn't simply replicate the documentation from numerical_approx()
?
comment:17 Changed 7 years ago by
One other detail which is wrong in the "at least" in "at least prec bits of precision". Certainly for the global numerical_approx()
function, it's exactly prec
bits.
comment:18 Changed 5 years ago by
 Keywords days74 added
 Milestone changed from sage6.5 to sage7.3
comment:19 Changed 5 years ago by
 Description modified (diff)
comment:20 Changed 5 years ago by
 Description modified (diff)
comment:21 Changed 5 years ago by
 Component changed from documentation to basic arithmetic
 Description modified (diff)
 Summary changed from Document default precision for .n() method to Refactor numerical_approx()
comment:22 Changed 5 years ago by
 Branch set to u/jdemeyer/document_default_precision_for__n___method
comment:23 Changed 5 years ago by
 Commit set to fafdb9c02ad5b9d0c7d85fe3cc4468015d08fd68
 Status changed from needs_work to needs_review
New commits:
fafdb9c  Improve numerical_approx

comment:24 followup: ↓ 25 Changed 5 years ago by
Most of the changes look good to me, but I don't like the new description of RealLiteral.numerical_approx()
: as far as I understand “real literals” are exact decimal numbers, so converting one to a RealField
element with d·log₁₀ 2 bits of precision doesn't really “change its precision” to d decimal digits.
And—just wondering, I have nothing against that choice—is there a reason why you didn't put the generic implementation directly in Element
(and put code to convert nonElements to Elements before calling it when necessary)?
comment:25 in reply to: ↑ 24 Changed 5 years ago by
Replying to mmezzarobba:
is there a reason why you didn't put the generic implementation directly in
Element
(and put code to convert nonElements to Elements before calling it when necessary)?
Because of the "put code to convert nonElements to Elements before calling it" part. I don't know how to do that in general.
comment:26 Changed 5 years ago by
ping
comment:27 Changed 5 years ago by
 Commit changed from fafdb9c02ad5b9d0c7d85fe3cc4468015d08fd68 to fd79227f589f50f190066f7e636356007544b556
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
fd79227  Improve numerical_approx

comment:28 Changed 5 years ago by
 Reviewers set to Marc Mezzarobba
 Status changed from needs_review to positive_review
I still disagree about the docstring of RealLiteral.numerical_approx()
, but let's not again delay the ticket by two months...
comment:29 Changed 5 years ago by
 Branch changed from u/jdemeyer/document_default_precision_for__n___method to fd79227f589f50f190066f7e636356007544b556
 Resolution set to fixed
 Status changed from positive_review to closed
I doubt that this is a bug. How would you guess the default precision for the
.n()
method?