Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prime_pi.plot is wrong (!) #6811

Closed
williamstein opened this issue Aug 23, 2009 · 10 comments
Closed

prime_pi.plot is wrong (!) #6811

williamstein opened this issue Aug 23, 2009 · 10 comments

Comments

@williamstein
Copy link
Contributor

I was computed Riemann's analytic formula for pi(X), and was disturbed it wasn't converging to pi(X). It turned out that the function in Sage for a while for plotting prime_pi is buggy! For example, try this:

sage: prime_pi.plot(5,10).show(gridlines='minor',frame=True)
sage: prime_pi(8)
4

You'll see a plot that has a horizontal line at height 5 on it.

This is very bad and embarrassing!

Component: number theory

Author: William Stein

Reviewer: R. Andrew Ohana, Minh Van Nguyen

Merged: Sage 4.1.2.alpha0

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

@williamstein williamstein added this to the sage-4.1.2 milestone Aug 23, 2009
@williamstein williamstein self-assigned this Aug 23, 2009
@williamstein
Copy link
Contributor Author

comment:1

I've attached code to fix this bug. It does things right.

(1) I created a plot_step_function command that can be used to plot general step functions, and added it to the reference manual.

(2) I changed the current broken prime_pi.plot to use that and use a much easier to understand (hence right) algorithm to generate the plot.

(3) I fixed a bunch of ReST mistakes in prime_pi.pyx while I was at it.

(4) I added prime_pi to the reference manual.

@williamstein
Copy link
Contributor Author

Attachment: trac_6811.patch.gz

@ohanar
Copy link
Member

ohanar commented Aug 23, 2009

comment:2

Looks fine:

  1. Importing sage.plot.all is no longer necessary in prime_pi.pyx
  2. May want to change
for i in range(len(v)):
    w.append(v[i])
    if i+1 < len(v):
        w.append((v[i+1][0],v[i][1]))

to

for i in range(len(v)-1):
    w.append(v[i])
    w.append((v[i+1][0],v[i][1]))
w.append(v[len(v)-1])

for readability.

  1. The plot_step_function always starts horizontal and ends vertically, this can sometimes lead to rather odd looking results in my opinion. For example, compare
sage: plot_step_function([(i,i^3) for i in range(6)])
sage: plot_step_function([(i,i^3) for i in range(6)]) + line([(5,125),(6,125)])

a. If we are to make any changes to this, we would need to consider uneven intervals of definition (say the function [(i<sup>2,i</sup>3) for i in range(6)]).

  1. Might be useful to use the plot_step_function elsewhere. For example, with Riemann sums it is either difficult or impossible to enable vertical lines, and the floor function is in the opposite situation.

@ohanar
Copy link
Member

ohanar commented Aug 23, 2009

comment:3

Also, we need to fix the credit situation in prime_pi.pyx

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 24, 2009

reviewer patch; fixes typos

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 24, 2009

comment:4

Attachment: trac_6811-reviewer.patch.gz

The reviewer patch trac_6811-reviewer.patch fixes some typos in trac_6811.patch. One of these typos results in a warning when building the reference manual.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 24, 2009

comment:5

Merged both patches.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 24, 2009

Author: William Stein

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 24, 2009

Merged: Sage 4.1.2.alpha0

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Aug 24, 2009

Reviewer: R. Andrew Ohana, Minh Van Nguyen

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Aug 24, 2009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants