sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 462 forks source link

Implement root numbers for elliptic curves over number fields #9320

Open arminstraub opened 14 years ago

arminstraub commented 14 years ago

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 commented 14 years ago

Author: Tim Dokchitser and group (Sage Days 22)

arminstraub commented 14 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,18 @@
 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.
JohnCremona commented 14 years ago
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 commented 14 years ago

Attachment: 9320_root_numbers.patch.gz

requires #9334 and #9684

arminstraub commented 14 years ago
comment:3

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

11d1fc49-71a1-44e1-869f-76be013245a0 commented 12 years ago

Rebased to 4.8.alpha5

11d1fc49-71a1-44e1-869f-76be013245a0 commented 12 years ago

Work Issues: fix ReST formatting

11d1fc49-71a1-44e1-869f-76be013245a0 commented 12 years ago
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.

f1c3b2e1-17d2-4698-9bf8-0710527ffd1a commented 12 years ago

Attachment: 9320_root_numbers-rebase_docscleaned.patch.gz

docstrings improved

f1c3b2e1-17d2-4698-9bf8-0710527ffd1a commented 12 years ago
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 commented 11 years ago
comment:6

apply only 9320_root_numbers-rebase_docscleaned.patch

fchapoton commented 11 years ago
comment:7

apply only 9320_root_numbers-rebase_docscleaned.patch

fchapoton commented 11 years ago

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

fchapoton commented 11 years ago
comment:8

seems to apply and pass all tests on 5.12.beta2

needs work to put coverage to 100%

fchapoton commented 10 years ago
comment:9

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

fchapoton commented 10 years ago
comment:10

Here is a git branch


New commits:

e63e7b29320: Implement root numbers for elliptic curves over number fields
fchapoton commented 10 years ago

Branch: u/chapoton/9320

fchapoton commented 10 years ago

Commit: e63e7b2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from e63e7b2 to 87938e0

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

87938e0trac #9320 trying to add documentation, still needs work
fchapoton commented 10 years ago
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 commented 10 years ago
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 commented 10 years ago
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) ?

JohnCremona commented 10 years ago
comment:15

Replying to @fchapoton:

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" ?

I'll find another example at 3. I don't know the code but I expect that it also depends on factors such as the ramification degree of the prime, so I'll give an example over Q(sqrt-3) with additive reduction at 3. Wait for it...

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

Yes

JohnCremona commented 10 years ago
comment:16

An example which is additive at 3:

sage: K.<r> = NumberField(x^2-x+1)
sage: E = EllipticCurve([r-1,r,1,r-1,-1])
sage: P3 = K.ideal(2*r-1)
sage: assert P3.is_prime() and P3.norm()==3
sage: assert E.has_additive_reduction(P3)
sage: assert P.root_number(P3)==1 ## not implemented

Here the value is taken from page 115 of my thesis, and agrees with Magma's value.

For additional testing we could put in an 'algorithm' parameter which could be 'magma' and then (of course) only work when Magma is installed, so you could have optional doctests conditional on Magma of the form

assert E.root_number(P) == E.root_number(P, algorithm='magma')
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

a0462dftrac #9320 work done on code and doc, but still one failing doctest
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 87938e0 to a0462df

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from a0462df to d40bf63

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

d40bf63trac #9320 doctest is indirect
fchapoton commented 10 years ago
comment:19

Thanks a lot for the examples. I have done what I could for this code, but there is now a failing doctest for the example you provided which has additive reduction at 2.

And I had to replace K.ideal(i+i) by K.ideal(1+i) ...

Not being in my field of expertise, I will now leave this ticket in more able hands.

chriswuthrich commented 10 years ago
comment:20

I had a quick look at this and there are indeed major issues with the local root number at places above 2 when the reduction is additive, potentially good. That is in the function _root_number_local_2 in ell_number_field.py.

With the current commit above we get an error for

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

because line 1966 tries to create and extension with a reducible polynomial K2 = K.extension(t**2+E.a2()*t+E.a4(), 'a2'). I guess - but it would take me some time to read the sources - that K2 should just be K in this case.

However there are other issues. For instance

sage: E = EllipticCurve([1+i,0,0,0,i])
sage: E._root_number_local_2(P2)

goes "bang".

I would think the code without the very bad case at places above 2 is ok and that could already go in. So I see two possible futures for this ticket:

  1. Someone fixes these issues above 2 or

  2. We raise an NotImplementedError if the reduction is additive and potentially good at a place above 2 and open a new ticket for fixing the problem.

I can help with 2), but I won't have time to do 1). But maybe there is someone out there who would.

JohnCremona commented 10 years ago
comment:21

It all started a long time ago, at the Sage Days & summer school at MSRI. Perhaps we good get Tim Dokchitser, who led the original project (and who developed the background theory, I think) involved? He works mainly in Magma, but back in 2010 he certainly was involved with this.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

b162b90Merge branch 'u/chapoton/9320' of trac.sagemath.org:sage into 9320
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from d40bf63 to b162b90

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from b162b90 to 6297587

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

6297587trac #9320 add missing import
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

fee78bbMerge branch 'u/chapoton/9320' into 6.10.b1
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from 6297587 to fee78bb

jdemeyer commented 8 years ago

Changed author from Tim Dokchitser and group (Sage Days 22) to Tim Dokchitser

chriswuthrich commented 8 years ago

Changed author from Tim Dokchitser to Tim Dokchitser and group (Sage Days 22)

chriswuthrich commented 8 years ago
comment:28

Sorry, I reverted that. Tim did not touch much of the code himself as far as I can remember that long long time ago in Berkley. I doubt he would want to vouch for this code by himself...

jdemeyer commented 8 years ago
comment:29

Instead of reverting, at least fix it.

jdemeyer commented 8 years ago

Changed author from Tim Dokchitser and group (Sage Days 22) to ???

chriswuthrich commented 8 years ago
comment:30

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

jdemeyer commented 8 years ago
comment:31

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

chriswuthrich commented 8 years ago
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 commented 8 years ago

Changed author from ??? to Armin Straub

fchapoton commented 8 years ago

Changed work issues from fix ReST formatting, coverage to none

fchapoton commented 8 years ago
comment:34

there are some failing doctests..

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

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