Opened 9 years ago

Closed 8 years ago

#12120 closed enhancement (fixed)

Improve documentation of numerical_approx()

Reported by: saraedum Owned by: mvngu
Priority: trivial Milestone: sage-5.3
Component: documentation Keywords: beginner
Cc: Merged in: sage-5.3.beta0
Authors: Julian Rueth Reviewers: Eviatar Bach, Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by kcrisman)

In a class on computer algebra, students were surprised by the behavior of the N() function. Here are some minimal examples that should illustrate the issues we ran into:

sage: x=N(pi,digits=3); x
3.14  
sage: y=N(3.14,digits=3); y
3.14  
sage: x==y
False
sage: N(pi,digits=1)
3.2
sage: N(pi-3,prec=2)
0.00

Imo, none of these is a bug, but I believe that the documentation should be improved.


Apply trac_12120_numerical_approx.patch and trac_12120-review.patch to the Sage library.

Attachments (2)

trac_12120-review.patch (1.5 KB) - added by kcrisman 9 years ago.
trac_12120_numerical_approx.patch (1.5 KB) - added by jdemeyer 8 years ago.
improve docstring of numerical_approx()

Download all attachments as: .zip

Change History (24)

comment:1 Changed 9 years ago by saraedum

The second example is also discussed in #11647.

comment:2 Changed 9 years ago by saraedum

  • Status changed from new to needs_review

comment:3 follow-up: Changed 9 years ago by eviatarbach

There's some other problems with the digits argument; it accepts non-integer values and zero. Can you add a sanity check?

comment:4 Changed 9 years ago by eviatarbach

The last example is odd; I made a thread about it at https://groups.google.com/forum/#!topic/sage-devel/nOdFzn6U06o.

comment:5 Changed 9 years ago by saraedum

  • Status changed from needs_review to needs_work

I'll try to incorporate what I've written on sage-devel into the patch.

comment:6 in reply to: ↑ 3 Changed 9 years ago by saraedum

Replying to eviatarbach:

There's some other problems with the digits argument; it accepts non-integer values and zero. Can you add a sanity check?

This ticket tries only to improve on the docstring of numerical_approx(). Adding sanity checks would make sense, though. I suggest you open a new ticket if you want to add sanity checks.

comment:7 Changed 9 years ago by saraedum

  • Status changed from needs_work to needs_review

comment:8 follow-up: Changed 9 years ago by eviatarbach

  • Status changed from needs_review to needs_work

Yes, I'll create another patch.

However, could you change the wording from "numerical instability"? This suggests there is something wrong. I like how Julian Rüth explained it: "This approximates both pi and 3 with two bits of precision and then computes their difference".

comment:9 Changed 9 years ago by saraedum

Ok. I have no clue about numerical analysis, but isn't that what numerical instability is about, i.e., addition not being numerically stable? Maybe I got the term wrong here. In any case, feel free to add this explanatory sentence.

comment:10 Changed 9 years ago by eviatarbach

I think "numerical instability" refers to propagation of errors in numerical algorithms, which is not what is occurring here. According to the sage-devel thread it is the intended behaviour, although perhaps unintuitive.

comment:11 in reply to: ↑ 8 Changed 9 years ago by saraedum

  • Authors changed from Julian Rüth to Julian Rueth
  • Keywords beginner added
  • Status changed from needs_work to needs_review

Replying to eviatarbach:

Yes, I'll create another patch.

However, could you change the wording from "numerical instability"? This suggests there is something wrong. I like how Julian Rüth explained it: "This approximates both pi and 3 with two bits of precision and then computes their difference".

Are you happy with the wording now?

comment:12 Changed 9 years ago by eviatarbach

Yes, looks good now. Sorry for delaying this.

comment:13 Changed 9 years ago by kcrisman

Hi, looks nice! This will be quite valuable. Note that Trac #11647 should probably use the new style of :trac:`11647`.

comment:14 Changed 9 years ago by saraedum

Ok. I changed Trac reference.

comment:15 Changed 9 years ago by kcrisman

  • Reviewers set to Eviatar Bach, Karl-Dieter Crisman

I like the following reviewer patch; to me, it clarifies one or two things. If you think I've misstated them, let me know. If it's fine with you, then click positive review!

Also, I think that #11647 probably should count as a dup of this. My guess is that there is no way to ensure at least one correct digit without occasionally bringing along a second for the ride.

Eviatar, feel free to open that sanity check ticket as well.

comment:16 Changed 9 years ago by kcrisman

  • Description modified (diff)

Patchbot: apply trac_12120_numerical_approx.patch and trac_12120-review.patch

comment:17 follow-up: Changed 9 years ago by saraedum

I would like to keep the examples without the *100. At least that's what my students ran into. The other changes you made are certainly an improvement.

comment:18 in reply to: ↑ 17 Changed 9 years ago by kcrisman

I would like to keep the examples without the *100. At least that's what my students ran into. The other changes you made are certainly an improvement.

Okay, I was just trying to make it clearer about the significant digits. Otherwise one could have imagined that digits really means digits, because it so happens that with pi, the number of digits equals the number of significant digits!

Changed 9 years ago by kcrisman

comment:19 Changed 9 years ago by kcrisman

  • Status changed from needs_review to positive_review

Okay, I just kept both! That can't hurt, and makes it clearer to me, at least, what's going on.

Nice work.

comment:20 Changed 9 years ago by saraedum

Sure. Thanks :)

Changed 8 years ago by jdemeyer

improve docstring of numerical_approx()

comment:21 Changed 8 years ago by jdemeyer

Re-created the patch using hg export (which you should always use to create a patch).

comment:22 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.3.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.