Opened 13 years ago

Closed 8 years ago

#6666 closed enhancement (fixed)

Implement analytic modular symbols for elliptic curves

Reported by: William Stein Owned by: William Stein
Priority: minor Milestone: sage-6.4
Component: modular forms Keywords: period, modular symbol
Cc: Peter Bruin, John Cremona Merged in:
Authors: William Stein, Peter Bruin Reviewers: Frédéric Chapoton, Peter Bruin
Report Upstream: N/A Work issues:
Branch: dcaefdc (Commits, GitHub, GitLab) Commit: dcaefdc03a500b6dcc9e3d683e7dc4bfd7e8a685
Dependencies: Stopgaps:

Status badges

Attachments (2)

trac_6666-part1.patch (9.3 KB) - added by William Stein 13 years ago.
trac_6666-rebased-5.12.patch (6.7 KB) - added by Frédéric Chapoton 9 years ago.

Download all attachments as: .zip

Change History (26)

Changed 13 years ago by William Stein

Attachment: trac_6666-part1.patch added

comment:1 Changed 9 years ago by David Loeffler

Component: number theorymodular forms
Report Upstream: N/A

comment:2 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:3 Changed 9 years ago by Frédéric Chapoton

Here is a rebased patch.

apply trac_6666-rebased-5.12.patch

comment:4 Changed 9 years ago by Frédéric Chapoton

Keywords: period modular symbol added
Status: needs_workneeds_info
Summary: [with patch] implement analytic modular symbols algorithm and cusp transformation matriximplement analytic modular symbols algorithm and cusp transformation matrix

comment:5 Changed 9 years ago by William Stein

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

Last edited 9 years ago by William Stein (previous) (diff)

comment:6 Changed 9 years ago by Frédéric Chapoton

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

comment:7 in reply to:  6 Changed 9 years ago by William Stein

Replying to chapoton:

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 :-)

comment:8 Changed 9 years ago by Frédéric Chapoton

Status: needs_infoneeds_review

ok, the bot has turned green. Needs review

comment:9 Changed 9 years ago by Frédéric Chapoton

new patch, rebased on 5.13.beta1

apply trac_6666-rebased-5.12.patch

comment:10 Changed 9 years ago by Frédéric Chapoton

Description: modified (diff)

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 ?

comment:11 Changed 9 years ago by Frédéric Chapoton

new patch, with numerical tolerance

apply trac_6666-rebased-5.12.patch

comment:12 Changed 9 years ago by Frédéric Chapoton

apply trac_6666-rebased-5.12.patch

Changed 9 years ago by Frédéric Chapoton

comment:13 Changed 9 years ago by Frédéric Chapoton

new patch, with lazy import

apply trac_6666-rebased-5.12.patch

comment:14 Changed 9 years ago by Frédéric Chapoton

Branch: u/chapoton/6666
Commit: 1986947ca4929007eaa5bdc06649ce79a93a41c3

New commits:

1986947trac #6666 -- implement analytic modular symbols algorithm and cusp transformation matrix

comment:15 Changed 9 years ago by git

Commit: 1986947ca4929007eaa5bdc06649ce79a93a41c32e06ddb568811cd552cc02695d2e8f87792ca9d8

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

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

comment:16 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:17 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:18 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:19 Changed 8 years ago by Peter Bruin

Cc: Peter Bruin added

comment:20 Changed 8 years ago by git

Commit: 2e06ddb568811cd552cc02695d2e8f87792ca9d891eb78f5c9d51048d4a9d5d36964719f4dc740b3

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)

comment:21 Changed 8 years ago by Peter Bruin

Authors: William Stein, Peter Bruin
Branch: u/chapoton/6666u/pbruin/6666-modular_symbol_numerical
Cc: John Cremona added
Commit: 91eb78f5c9d51048d4a9d5d36964719f4dc740b3b50bec1fbedb9ca734c47a5ab9ccb6ffee4bfa9a
Reviewers: Frédéric Chapoton, Peter Bruin
Summary: implement analytic modular symbols algorithm and cusp transformation matrixImplement analytic modular symbols for elliptic curves

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.

comment:22 Changed 8 years ago by Frédéric Chapoton

Branch: u/pbruin/6666-modular_symbol_numericalpublic/6666
Commit: b50bec1fbedb9ca734c47a5ab9ccb6ffee4bfa9adcaefdc03a500b6dcc9e3d683e7dc4bfd7e8a685
Status: needs_reviewpositive_review

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

comment:23 in reply to:  22 Changed 8 years ago by Peter Bruin

Replying to chapoton:

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.

comment:24 Changed 8 years ago by Volker Braun

Branch: public/6666dcaefdc03a500b6dcc9e3d683e7dc4bfd7e8a685
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.