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

Implement analytic modular symbols for elliptic curves #6666

Closed
williamstein opened this issue Aug 2, 2009 · 31 comments
Closed

Implement analytic modular symbols for elliptic curves #6666

williamstein opened this issue Aug 2, 2009 · 31 comments

Comments

@williamstein
Copy link
Contributor

Apply attachment: trac_6666-rebased-5.12.patch

CC: @pjbruin @JohnCremona

Component: modular forms

Keywords: period, modular symbol

Author: William Stein, Peter Bruin

Branch/Commit: dcaefdc

Reviewer: Frédéric Chapoton, Peter Bruin

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

@williamstein williamstein added this to the sage-5.11 milestone Aug 2, 2009
@williamstein williamstein self-assigned this Aug 2, 2009
@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Jul 25, 2013

comment:1

Attachment: trac_6666-part1.patch.gz

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@fchapoton
Copy link
Contributor

comment:3

Here is a rebased patch.

apply trac_6666-rebased-5.12.patch

@fchapoton
Copy link
Contributor

Changed keywords from none to period, modular symbol

@williamstein
Copy link
Contributor Author

comment:5

Frédéric Chapoton -- whoever you are -- I'm extremely happy seeing all the work you're doing on modular forms related functionality in Sage!!!!!!

+1000

-- William Stein

@fchapoton
Copy link
Contributor

comment:6

Thanks William. I am in algebra and combinatorics, not a number theorist, but I am trying to help nevertheless.

for the patchbot:

apply trac_6666-rebased-5.12.patch

@williamstein
Copy link
Contributor Author

comment:7

Replying to @fchapoton:

Thanks William. I am in algebra and combinatorics, not a number theorist, but I am trying to help nevertheless.

Thanks. I'm in number theory, not algebra/combinatorics, but I hope Sage has been helpful to people in algebra/combinatorics :-)

@fchapoton
Copy link
Contributor

comment:8

ok, the bot has turned green. Needs review

@fchapoton
Copy link
Contributor

comment:9

new patch, rebased on 5.13.beta1

apply trac_6666-rebased-5.12.patch

@fchapoton
Copy link
Contributor

comment:10

So there is numerical noise. Could somebody remind me what is the proper way to handle that ?

Use ... or use # rel tol or use # abs tol ?

@fchapoton

This comment has been minimized.

@fchapoton
Copy link
Contributor

comment:11

new patch, with numerical tolerance

apply trac_6666-rebased-5.12.patch

@fchapoton
Copy link
Contributor

comment:12

apply trac_6666-rebased-5.12.patch

@fchapoton
Copy link
Contributor

Attachment: trac_6666-rebased-5.12.patch.gz

@fchapoton
Copy link
Contributor

comment:13

new patch, with lazy import

apply trac_6666-rebased-5.12.patch

@fchapoton
Copy link
Contributor

Commit: 1986947

@fchapoton
Copy link
Contributor

Branch: u/chapoton/6666

@fchapoton
Copy link
Contributor

New commits:

[1986947](https://github.com/sagemath/sagetrac-mirror/commit/1986947)trac #6666 -- implement analytic modular symbols algorithm and cusp transformation matrix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 4, 2014

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

2e06ddbtrac #6666 -- implement analytic modular symbols algorithm and cusp transformation matrix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 4, 2014

Changed commit from 1986947 to 2e06ddb

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 20, 2014

Changed commit from 2e06ddb to 91eb78f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 20, 2014

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

28666a8Merge branch 'u/chapoton/6666' of trac.sagemath.org:sage into 6.5.b3
91eb78ftrac #6666 minor formatting (towards pep8)

@pjbruin
Copy link
Contributor

pjbruin commented Dec 22, 2014

Author: William Stein, Peter Bruin

@pjbruin
Copy link
Contributor

pjbruin commented Dec 22, 2014

comment:21

The most important part of this ticket (periods of newforms) was implemented in a more general context (and with better precision handling) in #11215. The new commit reorganises the code, moves it to a more logical place (in my opinion) and uses the period() method of #11215.

@pjbruin
Copy link
Contributor

pjbruin commented Dec 22, 2014

Changed commit from 91eb78f to b50bec1

@pjbruin
Copy link
Contributor

pjbruin commented Dec 22, 2014

Reviewer: Frédéric Chapoton, Peter Bruin

@pjbruin
Copy link
Contributor

pjbruin commented Dec 22, 2014

Changed branch from u/chapoton/6666 to u/pbruin/6666-modular_symbol_numerical

@pjbruin pjbruin changed the title implement analytic modular symbols algorithm and cusp transformation matrix Implement analytic modular symbols for elliptic curves Dec 22, 2014
@fchapoton
Copy link
Contributor

Changed branch from u/pbruin/6666-modular_symbol_numerical to public/6666

@fchapoton
Copy link
Contributor

comment:22

Looks good to me.

I allowed myself a few minor changes.


New commits:

4ba47c6Merge branch 'u/pbruin/6666-modular_symbol_numerical' into 6.5.b5
dcaefdctrac #6666 some pep8 details + crossref

@fchapoton
Copy link
Contributor

Changed commit from b50bec1 to dcaefdc

@pjbruin
Copy link
Contributor

pjbruin commented Jan 24, 2015

comment:23

Replying to @fchapoton:

Looks good to me.

Thanks for the review!

I allowed myself a few minor changes.

I'm not disputing your changes to the whitespace here, but note that PEP 8 does not say that there should be spaces around all operators, only the relational ones. From https://www.python.org/dev/peps/pep-0008/:

If operators with different priorities are used, consider adding whitespace around the operators with the lowest priority(ies). Use your own judgment; however, never use more than one space, and always have the same amount of whitespace on both sides of a binary operator.
Yes:

i = i + 1
submitted += 1
x = x*2 - 1
hypot2 = x*x + y*y
c = (a+b) * (a-b)

No:

i=i+1
submitted +=1
x = x * 2 - 1
hypot2 = x * x + y * y
c = (a + b) * (a - b)

Actually, in the case of c = ..., I would personally prefer the "no" option or even (a + b)*(a - b), which is closer to standard mathematical typesetting, but in any case this is a matter of taste.

@vbraun
Copy link
Member

vbraun commented Jan 25, 2015

Changed branch from public/6666 to dcaefdc

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

5 participants