Refactor numerical_approx()

[sagetrac-dsm]

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.

CC: @benjaminfjones

Component: basic arithmetic

Keywords: numerical_approx, sd40.5, days74

Author: Jeroen Demeyer

Branch: fd79227

Reviewer: Marc Mezzarobba

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

[sagetrac-dsm]

Changed keywords from numerical_approx to numerical_approx, sd40.5

[jdemeyer]

comment:7

I doubt that this is a bug. How would you guess the default precision for the .n() method?

[kcrisman]

comment:11

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!)

[jdemeyer]

comment:12

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.

[jdemeyer]

comment:13

About hold=True, that even doesn't seem to work with real numbers:

sage: gamma(1.2345678901234567890, hold=True)
0.9097200568693150729

[kcrisman]

comment:14

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!

[jdemeyer]

comment:17

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.

[jdemeyer]

Changed keywords from numerical_approx, sd40.5 to numerical_approx, sd40.5, days74

[jdemeyer]

Author: Jeroen Demeyer

[jdemeyer]

Branch: u/jdemeyer/document_default_precision_for__n___method

[jdemeyer]

Commit: fafdb9c

[jdemeyer]

New commits:

fafdb9c

Improve numerical_approx

[mezzarobba]

comment:24

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 non-Elements to Elements before calling it when necessary)?

[jdemeyer]

comment:25

Replying to @mezzarobba:

is there a reason why you didn't put the generic implementation directly in Element (and put code to convert non-Elements to Elements before calling it when necessary)?

Because of the "put code to convert non-Elements to Elements before calling it" part. I don't know how to do that in general.

[jdemeyer]

comment:26

ping

[sagetrac-git]

Changed commit from fafdb9c to fd79227

[sagetrac-git]

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

fd79227

Improve numerical_approx

[mezzarobba]

Reviewer: Marc Mezzarobba

[mezzarobba]

comment:28

I still disagree about the docstring of RealLiteral.numerical_approx(), but let's not again delay the ticket by two months...

[vbraun]

Changed branch from u/jdemeyer/document_default_precision_for__n___method to fd79227

[fchapoton]

Changed commit from fd79227 to none