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

cuspform_lseries() method for a new form has precision issues #16916

Closed
tornaria opened this issue Sep 1, 2014 · 17 comments
Closed

cuspform_lseries() method for a new form has precision issues #16916

tornaria opened this issue Sep 1, 2014 · 17 comments

Comments

@tornaria
Copy link
Contributor

tornaria commented Sep 1, 2014

sage: f = Newforms(67, names='a')[0]
sage: L = f.cuspform_lseries()      
...
RuntimeError: Unable to create L-series, due to precision or other limits in PARI.

This is because the field prec is used both for the numerical precision and for the number of coefficients computed.

Since the L-functions class has a num_coeffs() method, the proper thing is to use that to know how many coefficients to compute.

Component: modular forms

Keywords: lseries

Author: Gonzalo Tornaría

Branch/Commit: 13dbf33

Reviewer: Michael Neururer

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

@tornaria tornaria added this to the sage-6.4 milestone Sep 1, 2014
@tornaria
Copy link
Contributor Author

tornaria commented Sep 1, 2014

Commit: 74b6a67

@tornaria
Copy link
Contributor Author

tornaria commented Sep 1, 2014

Branch: u/tornaria/16916

@tornaria
Copy link
Contributor Author

tornaria commented Sep 1, 2014

comment:1

Fixed as proposed.

See also #11631 (cusp forms of degree > 1) and #12015 (more general implementation of L-series).


New commits:

74b6a67Trac #16916: fix precision issues in cuspform_lseries()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 1, 2014

Changed commit from 74b6a67 to ae26b4a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 1, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

ae26b4aTrac #16916: avoid precision issues in cuspform_lseries

@tornaria
Copy link
Contributor Author

tornaria commented Sep 1, 2014

comment:3

After the first commit there were a few cases for which the runtime error would still happen. It turns out that

f.q_expansion(num_coeff).list()

is not always a list of length num_coeff, because trailing zeros are removed. Instead we now use

f.q_expansion(num_coeff).padded_list()

which will not drop the trailing zeros.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 1, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

13dbf33remove tabs

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 1, 2014

Changed commit from ae26b4a to 13dbf33

@sagetrac-mneururer
Copy link
Mannequin

sagetrac-mneururer mannequin commented Sep 1, 2014

comment:5

Review: This patch fixes the problem in the description. I tried to calculate L(f,1) for new forms of various levels but even setting the precision very high I was never able to calculate with levels above 100. With the patch I calculated L(f,1) for all new forms with rational coefficients up to level 400 without increasing the precision.

@sagetrac-mneururer
Copy link
Mannequin

sagetrac-mneururer mannequin commented Sep 1, 2014

Reviewer: Michael Neururer

@vbraun
Copy link
Member

vbraun commented Sep 6, 2014

comment:8

Author name should be full name, not trac username.

@tornaria
Copy link
Contributor Author

tornaria commented Sep 7, 2014

Changed author from tornaria to Gonzalo Tornaría

@tornaria
Copy link
Contributor Author

tornaria commented Sep 7, 2014

comment:9

Replying to @vbraun:

Author name should be full name, not trac username.

Isn't that inconsistent with how the "Reported by" field is handled?
Also, the "reported by" field links to a query for tickets reported by that person, but the "authors" field doesn't.

By the way, this ticket and #11631 are sooo embarrassing that I hope they can make it into 6.4... (especially since the fixes where kind of trivial).

@vbraun
Copy link
Member

vbraun commented Sep 7, 2014

comment:10

"reported by" is where trac stores the account that created the ticket.

Author/Reviewer are not necessarily trac accounts (could have been written by somebody without an account, for example). Also, they end up in the git commit log.

@tornaria
Copy link
Contributor Author

tornaria commented Sep 7, 2014

comment:11

Replying to @vbraun:

"reported by" is where trac stores the account that created the ticket.

Yes, I know, but with the same reason a defect could have been reported by somebody without an account and picked up by someone different.

Author/Reviewer are not necessarily trac accounts (could have been written by somebody without an account, for example). Also, they end up in the git commit log.

So anybody can change the !Author/Reviewer fields without a trac account?

I agree that git commit logs should have full name and email (mine do).

I understand your point, but I still think it's a bit inconsistent, and makes it difficult to search for tickets by name, because the account name is used for some fields (e.g. reported by, comments) but the real name is used for others (author, reviewer). Also, real names are not always canonical (e.g. mine has an accented i and it's not always used consistently -- can't be used in sage source code, for instance), while trac login names should be unique.

Anyway, thanks for your work.

@vbraun
Copy link
Member

vbraun commented Sep 7, 2014

comment:12

You need a trac account to edit tickets but you might post somebody else's code.

You can use UTF8 in the sources, this should be enough to write your name in your preferred spelling. If you can't decide how to spell your name I can't help you, though ;-)

@vbraun
Copy link
Member

vbraun commented Sep 8, 2014

Changed branch from u/tornaria/16916 to 13dbf33

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