Opened 9 years ago

Last modified 4 months ago

#9320 needs_work enhancement

Implement root numbers for elliptic curves over number fields

Reported by: arminstraub Owned by: cremona
Priority: minor Milestone:
Component: elliptic curves Keywords: root number
Cc: was, cturner, beankao, pbruin, cremona Merged in:
Authors: Armin Straub Reviewers:
Report Upstream: N/A Work issues:
Branch: public/9320 (Commits) Commit: 82d2ddc38912cf9d23b5783444753e060149a740
Dependencies: Stopgaps:

Description (last modified by arminstraub)

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.

Attachments (3)

9320_root_numbers.patch (13.9 KB) - added by arminstraub 9 years ago.
requires #9334 and #9684
9320_root_numbers-rebase.patch (13.6 KB) - added by davidloeffler 7 years ago.
Rebased to 4.8.alpha5
9320_root_numbers-rebase_docscleaned.patch (14.5 KB) - added by cturner 7 years ago.
docstrings improved

Download all attachments as: .zip

Change History (50)

comment:1 Changed 9 years ago by arminstraub

  • Authors set to Tim Dokchitser and group (Sage Days 22)
  • Cc beankao added
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by cremona

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

Changed 9 years ago by arminstraub

requires #9334 and #9684

comment:3 Changed 9 years ago by arminstraub

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

Changed 7 years ago by davidloeffler

Rebased to 4.8.alpha5

comment:4 Changed 7 years ago by davidloeffler

  • Status changed from needs_review to needs_work
  • Work issues set to fix ReST formatting

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.

Changed 7 years ago by cturner

docstrings improved

comment:5 Changed 7 years ago by cturner

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?

comment:6 Changed 6 years ago by chapoton

apply only 9320_root_numbers-rebase_docscleaned.patch

comment:7 Changed 6 years ago by chapoton

apply only 9320_root_numbers-rebase_docscleaned.patch

comment:8 Changed 6 years ago by chapoton

  • Work issues changed from fix ReST formatting to fix ReST formatting, coverage

seems to apply and pass all tests on 5.12.beta2

needs work to put coverage to 100%

comment:9 Changed 5 years ago by chapoton

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

comment:10 Changed 5 years ago by chapoton

  • Branch set to u/chapoton/9320
  • Commit set to e63e7b2d1cb4f4d23e2c8b527ce3c9cdf2cf0cbe

Here is a git branch


New commits:

e63e7b29320: Implement root numbers for elliptic curves over number fields

comment:11 Changed 5 years ago by git

  • Commit changed from e63e7b2d1cb4f4d23e2c8b527ce3c9cdf2cf0cbe to 87938e0bdccc397f9527b1db39b9c85006f40232

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

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

comment:12 Changed 5 years ago by chapoton

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 ?

comment:13 Changed 5 years ago by cremona

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.

comment:14 follow-up: Changed 5 years ago by chapoton

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

comment:15 in reply to: ↑ 14 Changed 5 years ago by cremona

Replying to chapoton:

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

comment:16 Changed 5 years ago by cremona

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

comment:17 Changed 5 years ago by git

  • Commit changed from 87938e0bdccc397f9527b1db39b9c85006f40232 to a0462dfb7aae331e112ed2979d7fffbd99319fc4

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

a0462dftrac #9320 work done on code and doc, but still one failing doctest

comment:18 Changed 5 years ago by git

  • Commit changed from a0462dfb7aae331e112ed2979d7fffbd99319fc4 to d40bf637722cd2cb68b1ce0310290bbed2f7a1f5

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

d40bf63trac #9320 doctest is indirect

comment:19 Changed 5 years ago by chapoton

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.

comment:20 Changed 5 years ago by wuthrich

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
  1. 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.

comment:21 Changed 5 years ago by cremona

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.

comment:22 Changed 5 years ago by pbruin

  • Cc pbruin added

comment:23 Changed 4 years ago by git

  • Commit changed from d40bf637722cd2cb68b1ce0310290bbed2f7a1f5 to b162b900fb9213f5fde451d69211fcf1f5ad25bf

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

b162b90Merge branch 'u/chapoton/9320' of trac.sagemath.org:sage into 9320

comment:24 Changed 4 years ago by git

  • Commit changed from b162b900fb9213f5fde451d69211fcf1f5ad25bf to 62975879c068b50731fe3faca2280b68acaa5b71

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

6297587trac #9320 add missing import

comment:25 Changed 4 years ago by git

  • Commit changed from 62975879c068b50731fe3faca2280b68acaa5b71 to fee78bb780730a9089eb7ed776d71d7d5e515263

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

fee78bbMerge branch 'u/chapoton/9320' into 6.10.b1

comment:26 Changed 4 years ago by jdemeyer

  • Authors changed from Tim Dokchitser and group (Sage Days 22) to Tim Dokchitser

comment:27 Changed 4 years ago by wuthrich

  • Authors changed from Tim Dokchitser to Tim Dokchitser and group (Sage Days 22)

comment:28 Changed 4 years ago by wuthrich

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...

comment:29 Changed 4 years ago by jdemeyer

  • Authors changed from Tim Dokchitser and group (Sage Days 22) to ???

Instead of reverting, at least fix it.

comment:30 Changed 4 years ago by wuthrich

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

comment:31 Changed 4 years ago by jdemeyer

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

comment:32 Changed 4 years ago by wuthrich

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.

comment:33 Changed 4 years ago by jdemeyer

  • Authors changed from ??? to Armin Straub

comment:34 Changed 4 years ago by chapoton

  • Work issues fix ReST formatting, coverage deleted

there are some failing doctests..

comment:35 Changed 3 years ago by git

  • Commit changed from fee78bb780730a9089eb7ed776d71d7d5e515263 to b4f448ced7e76f9285106f40a5054b1258f19851

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

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

comment:36 Changed 3 years ago by git

  • Commit changed from b4f448ced7e76f9285106f40a5054b1258f19851 to bd9aa7fdcaf2ac0857e42820281bd9090c5d2be0

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

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

comment:37 Changed 3 years ago by git

  • Commit changed from bd9aa7fdcaf2ac0857e42820281bd9090c5d2be0 to 28cb70ca84dc378dcbd446a53fc671c7e4efdcf3

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

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

comment:38 Changed 3 years ago by chapoton

  • Cc cremona added

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

comment:39 Changed 3 years ago by wuthrich

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.

comment:40 Changed 3 years ago by git

  • Commit changed from 28cb70ca84dc378dcbd446a53fc671c7e4efdcf3 to 7a131d0b685656947d273c4e9de30bab69408935

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

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

comment:41 Changed 3 years ago by chapoton

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.

comment:42 Changed 3 years ago by git

  • Commit changed from 7a131d0b685656947d273c4e9de30bab69408935 to 60e9a78c6facda896d1ec681c7b429d3b832bdc8

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

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

comment:43 Changed 21 months ago by git

  • Commit changed from 60e9a78c6facda896d1ec681c7b429d3b832bdc8 to 66f60b95d17439eb53b8dea2a9dde34963b939bd

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

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

comment:44 Changed 17 months ago by git

  • Commit changed from 66f60b95d17439eb53b8dea2a9dde34963b939bd to c34b3080820198115a79dee956550acc5a495f34

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

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

comment:45 Changed 5 months ago by cremona

  • Branch changed from u/chapoton/9320 to public/9320
  • Commit changed from c34b3080820198115a79dee956550acc5a495f34 to 223f82090ca704dd4ae71715eb661adf727b6522

Merged with 8.6.beta1


New commits:

223f820Merge branch 'develop' into 9320

comment:46 Changed 4 months ago by git

  • Commit changed from 223f82090ca704dd4ae71715eb661adf727b6522 to 82d2ddc38912cf9d23b5783444753e060149a740

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

82d2ddcfix indentation error in previous commit

comment:47 Changed 4 months ago by cremona

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.

Note: See TracTickets for help on using tickets.