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 )
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)
Change History (24)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
- Status changed from new to needs_review
comment:3 follow-up: ↓ 6 Changed 9 years ago by
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
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
- 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
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
- Status changed from needs_work to needs_review
comment:8 follow-up: ↓ 11 Changed 9 years ago by
- 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
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
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
- 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
Yes, looks good now. Sorry for delaying this.
comment:13 Changed 9 years ago by
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
Ok. I changed Trac reference.
comment:15 Changed 9 years ago by
- 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
- Description modified (diff)
Patchbot: apply trac_12120_numerical_approx.patch and trac_12120-review.patch
comment:17 follow-up: ↓ 18 Changed 9 years ago by
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
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
comment:19 Changed 9 years ago by
- 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
Sure. Thanks :)
comment:21 Changed 8 years ago by
Re-created the patch using hg export
(which you should always use to create a patch).
comment:22 Changed 8 years ago by
- Merged in set to sage-5.3.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
The second example is also discussed in #11647.