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 root numbers for elliptic curves over number fields #9320

Open
arminstraub opened this issue Jun 23, 2010 · 72 comments
Open

Implement root numbers for elliptic curves over number fields #9320

arminstraub opened this issue Jun 23, 2010 · 72 comments

Comments

@arminstraub
Copy link

Root numbers for elliptic curves are currently only implemented via Pari (pari(E).ellrootno()) and only over the rational numbers.

We (Tim Dokchitser's group at Sage Days 22) intend to add the possibility to compute local and global root numbers for elliptic curves over number fields. A first patch may not fully implement the case of additive reduction over primes dividing 2 or 3.

Update: Attached is a first implementation which allows for instance:

sage: K.<a>=NumberField(x^4+2)
sage: E = EllipticCurve(K, [1, a, 0, 1+a, 0])
sage: E.root_number()
1
sage: E.root_number(K.ideal(a+1))
1

Note that the implementation needs the patches #9334 (Hilbert symbol) and #9684 ("dirty model") to be applied.

To prevent incorrect results in some cases as well as crashes, the tickets #9389 and #9417 need to be addressed.

CC: @williamstein @sagetrac-cturner @sagetrac-beankao @pjbruin @JohnCremona

Component: elliptic curves

Keywords: root number

Author: Armin Straub

Branch/Commit: public/9320 @ 82d2ddc

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

@arminstraub
Copy link
Author

Author: Tim Dokchitser and group (Sage Days 22)

@arminstraub

This comment has been minimized.

@JohnCremona
Copy link
Member

comment:2

I don't think it is possible to review this until the ticket it depends on (#9334) which needs work has been fixed.

@arminstraub
Copy link
Author

Attachment: 9320_root_numbers.patch.gz

requires #9334 and #9684

@arminstraub
Copy link
Author

comment:3

Adapted the patch to reflect the renaming of "tidy" to "reduce" following #9684.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Dec 30, 2011

Rebased to 4.8.alpha5

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Dec 30, 2011

Work Issues: fix ReST formatting

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Dec 30, 2011

comment:4

Attachment: 9320_root_numbers-rebase.patch.gz

Ticket #9334 has been merged, so this is now ready for review. Sadly it fails to apply, due to a trivial conflict with #11749. I've uploaded a rebased patch, and checked that all doctests in sage/schemes/elliptic_curves pass with this applied.

However, some (trivial but tedious) work is needed fixing the ReST formatting of the docstrings -- the indentation is all over the place, and :: should only be used to introduce example code blocks.

@loefflerd loefflerd mannequin added s: needs work and removed s: needs review labels Dec 30, 2011
@sagetrac-cturner
Copy link
Mannequin

sagetrac-cturner mannequin commented Jan 13, 2012

Attachment: 9320_root_numbers-rebase_docscleaned.patch.gz

docstrings improved

@sagetrac-cturner
Copy link
Mannequin

sagetrac-cturner mannequin commented Jan 13, 2012

comment:5

I have tried to clean this up, but I'm not very experienced so I may have made some mistakes or missed something - could someone please have a look and help me to get it right?

@fchapoton
Copy link
Contributor

comment:6

apply only 9320_root_numbers-rebase_docscleaned.patch

@fchapoton
Copy link
Contributor

comment:7

apply only 9320_root_numbers-rebase_docscleaned.patch

@fchapoton
Copy link
Contributor

Changed work issues from fix ReST formatting to fix ReST formatting, coverage

@fchapoton
Copy link
Contributor

comment:8

seems to apply and pass all tests on 5.12.beta2

needs work to put coverage to 100%

@fchapoton
Copy link
Contributor

comment:9

this one just needs a little more doc (three functions need doctests)

@fchapoton
Copy link
Contributor

comment:10

Here is a git branch


New commits:

e63e7b29320: Implement root numbers for elliptic curves over number fields

@fchapoton
Copy link
Contributor

Branch: u/chapoton/9320

@fchapoton
Copy link
Contributor

Commit: e63e7b2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 16, 2014

Changed commit from e63e7b2 to 87938e0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 16, 2014

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

87938e0trac #9320 trying to add documentation, still needs work

@fchapoton
Copy link
Contributor

comment:12

It would be good if some expert of elliptic curve could provide correct doctests for the local root numbers at primes 2 and 3. Could you have a look, please ?

@JohnCremona
Copy link
Member

comment:13

I suggest that a good source of examples would be elliptic curves over number fields where we know the associated modular form, since the root number at a bad prime should match the Atkin-Lehner eigenvalue. (The alternative would be to compue a whole lot of examples with Magma, but that would make me uncomfortable; nevertheless we should of course check that our results are compatible with Magma.)

There is no issue when the primes have multiplicative reduction, since then the root number is very easy being minus E.ap, i.e. depends only on whether the number of points on the reduction is p+1 or p-1 (of course "p" means Norm(p) in the number field case). It's the case of additive reduction at primes dividing 2 or 3 which are harder.

Here is one taken from my thesis (see http://www.numdam.org/numdam-bin/search?h=nc&id=CM_1984__51_3_275_0&format=complete):

K.<i>=QuadraticField(-1)
E=EllipticCurve([1+i, 1+i, 0,i,0])
P2=K.ideal(i+i)
E.root_number(P2)
-1

which I checked with Magma. The conductor here is P2^2 * P41.

Is this what you want? How many examples do you need?

Tables of elliptic curves over number fields do exist, and were in fact one of the topics of last week's Curves and Automorphic Forms workshop in Arizona.

@fchapoton
Copy link
Contributor

comment:14

One example would be enough, I think if it is bad at both 2 and 3. Maybe one can just use the one above as "indirect doctest" ?

Do you really mean K.ideal(i+i) ?

@chriswuthrich
Copy link
Contributor

comment:30

What is the issue ? Why can't you leave the original string in there ?

@jdemeyer
Copy link

comment:31

Because "Tim Dokchitser and group (Sage Days 22)" isn't an actual person.

@chriswuthrich
Copy link
Contributor

comment:32

But it was a collaborative effort. The wiki for the sage days lists the participants in this group as "Armin, Charlie, Hatice, Christ, Lola, Robert Miller, Thilina, M. Tip, Robert Bradshaw " I am not sure who actually did the coding and I don't remember all full names. So the original string was probably the closest to who the author was. Otherwise set it to Armin Straub, who did the original uploading onto this trac ticket.

@jdemeyer
Copy link

Changed author from ??? to Armin Straub

@fchapoton
Copy link
Contributor

Changed work issues from fix ReST formatting, coverage to none

@fchapoton
Copy link
Contributor

comment:34

there are some failing doctests..

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2016

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

b4f448cMerge branch 'u/chapoton/9320' into 7.1.b5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2016

Changed commit from fee78bb to b4f448c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 5, 2016

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

3e3f4aaMerge branch 'u/chapoton/9320' into 7.1.b6
bd9aa7ftrac #9320 failing doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 5, 2016

Changed commit from b4f448c to bd9aa7f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 5, 2016

Changed commit from bd9aa7f to 28cb70c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 5, 2016

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

28cb70ctrac #9320 remove the algo keyword and the duplicate code

@fchapoton
Copy link
Contributor

comment:38

Could some expert in elliptic curves have a look at the last failing doctest, please ?

@chriswuthrich
Copy link
Contributor

comment:39

I can simply repeat my comment:20 above. This is a genuine bug.

My favourite solution is still to raise non implemented warnings for the cases this code does not do currently.

Before accepting this ticket, the reviewer will have to do lots of comparison with magma. The code in magma will have plenty less bugs than this one as the Dokchitsers have worked a lot on that code.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 10, 2016

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

7a131d0trac #9320 trying to fix the issues for potential good reduction over 2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 10, 2016

Changed commit from 28cb70c to 7a131d0

@fchapoton
Copy link
Contributor

comment:41

I tried to fix the problem, but only with partial success. It seems that at some point
the code assumes vanishing of coeff a1 in long Weierstrass equation, and I cannot see why.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 29, 2016

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

60e9a78Merge branch 'u/chapoton/9320' in 7.3.rc0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 29, 2016

Changed commit from 7a131d0 to 60e9a78

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2017

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

66f60b9Merge branch 'u/chapoton/9320' in 8.1.b5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2017

Changed commit from 60e9a78 to 66f60b9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 4, 2018

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

c34b308Merge branch 'u/chapoton/9320' in 8.2.b2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 4, 2018

Changed commit from 66f60b9 to c34b308

@JohnCremona
Copy link
Member

comment:45

Merged with 8.6.beta1


New commits:

223f820Merge branch 'develop' into 9320

@JohnCremona
Copy link
Member

Changed commit from c34b308 to 223f820

@JohnCremona
Copy link
Member

Changed branch from u/chapoton/9320 to public/9320

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 9, 2019

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

82d2ddcfix indentation error in previous commit

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 9, 2019

Changed commit from 223f820 to 82d2ddc

@JohnCremona
Copy link
Member

comment:47

After merging with 8.6.beta1 I ran the tests, and there is one failure in line 2294 of ell_number_field where a local root number at a ramified prime above 2 where there is additive reduction is computed incorrectly.

I checked with Magma that -1 is the correct value. This curve is http://beta.lmfdb.org/EllipticCurve/2.0.4.1/164.2/a/2 and has associated Bianchi newform http://beta.lmfdb.org/ModularForm/GL2/ImaginaryQuadratic/2.0.4.1/164.2/a/ where you can see that the Atkin-Lehner eigenvalue at 1+i is -1, giving a second independent evaluation.

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