Opened 9 years ago

Closed 8 years ago

Improve documentation of numerical_approx()

Reported by: Owned by: saraedum mvngu trivial sage-5.3 documentation beginner sage-5.3.beta0 Julian Rueth Eviatar Bach, Karl-Dieter Crisman N/A

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.

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: ↓ 6 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: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

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: ↓ 11 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
• Status changed from needs_work to needs_review

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: ↓ 18 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!

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.

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.