make -1 modular symbols from eclib available to elliptic curves

[chriswuthrich]

This ticket is a follw-up from #10236. In there certain changes from #9476 and #9247 were reverted back. Here is what has to be done to make modular symbols with negative sign available for computations with elliptic curves, e.g. p-adic L-series.

Currently eclib can compute the space of modular symbols of sign = -1, but ECModularSymbol in sage.libs.cremona.newforms.pyx can not use them yet to compute {0,r}-

Once this is there, one can modify sage.schemes.elliptic_curves.ell_modular_symbols.py to allow sign = -1 in ModularSymbolECLIB. (Where one has to think a little bit about the normalization.) This would allow to switch in line 1098 of ell_rational_field.py and in line 131 of padic_lseries.py to make use_eclib=True to be default.

Depends on #22077

CC: @JohnCremona

Component: elliptic curves

Keywords: modular symbols

Author: John Cremona

Branch/Commit: 49be7d8

Reviewer: Chris Wuthrich

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

[JohnCremona]

comment:1

The current eclib (eclib-20100711.spkg) has normalization problems in the "sign=-1" modular symbols, which I am currently working on fixing. The Sage interface should perhaps wait until that is done.

[chriswuthrich]

comment:6

Ping ?

I was working on #20864 when I changed the plus symbols to be computed by eclib by default. I wondered if the negative could be added now? Sage normalises the negative ones correctly now. We only need some rational multiple from eclib.

[JohnCremona]

comment:7

Implementing this just went up my to-do list, after hearing a lecture by Mazur on his experimental work with Rubin which makes heavy use of this modular symbol functionality. Karl told me that it would save them a lot of time for minus symbols to be available from eclib. As soon as I can, I will.

[chriswuthrich]

comment:8

Excellent. I will help of course.

I am curious what Rubin and Mazur are up to, too.

[JohnCremona]

comment:9

I will make this ticket dependent on another which will upgrade the eclib version in Sage to one which will be released later today. Two reasons: (1) I fixed a bug which sometimes caused the plus symbols to all be mutiplied by a factor >1 (in known examples, a factor of 2). (2) I have added a facility to eclib to base symbols at infinity (oo) instead of 0, i.e. it computes the image of either {0,r} or {oo,r} for a given rational.

Example:

sage: E=EllipticCurve("121b1")
sage: M=E.modular_symbol(1)
Warning : Could not normalize the modular symbols, maybe all further results will be multiplied by -1, 2 or -2.
sage: M(1/7)
-2

Here the error message is coming from Sage, not eclib. And the correct value is 1 though the current eclib version returns 2.

[JohnCremona]

comment:10

See #22077

[JohnCremona]

Dependencies: #22077

[JohnCremona]

comment:11

#22077 is now ready for review. I will base by branch for this ticket on that one, so am now setting that to be a prerequisite to this.

[JohnCremona]

Author: John Cremona

[JohnCremona]

Branch: u/cremona/10256

[JohnCremona]

comment:12

I have set this to Needs Review but note that #22077 (upgrading eclib to version v20161230) is a dependency and is currently also waiting review.

[JohnCremona]

Commit: 6637227

[chriswuthrich]

comment:13

I am working on this. I spotted a few misprints, but more seriously:

• This code also changes the normalization for modular symbols in sage. In particular, the warning for 121b1 is no longer there, despite the function returning the wrong result.

sage: E = EllipticCurve("121b1")
sage: m = E.modular_symbol(implementation="sage")
sage: m(1/7)
-1/2
sage: me = E.modular_symbol()
sage: me(1/7)
1/2

• The second thing is that there are more places which need to change taking eclib automatically for negative symbols. Like line 219 in padic_lseries.py

[JohnCremona]

comment:18

The previous comment needs correcting since I was testing on the wrong machine and not actually useing the latest eclib version v20161230. Now eclib's ecnf program maps {oo,-1/3} to (-3/5,+1) as it should for consistency. But consistency is not correctness, and in fact eclib only normalizes (w.r.t. sign) the plus symbols, and then only when L(E,1) is non-zero.

Hence there is more to be done within eclib to get the signs right. I have set #22077 back to 'needs work' but it would also be possible for the sign to be checked on the Sage side.

[JohnCremona]

comment:19

There's a new version of eclib (v20170104) over at #22077 and my last branch on this ticket (u/cremona/10256) works OK with it. I have not yet merged and tested u/wuthrich/ticket/10256 but I did test with wuthrich's independent test script.

[sagetrac-git]

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

7e695e8	updated eclib to v20170103
777fa61	Merge branch 'u/cremona/22077' of git://trac.sagemath.org/sage into eclib
e101d21	trac #10256: references
8fabe91	updated eclib to v20170104
1c78871	updated eclib to v20170104 again
b12bc13	Merge branch 'u/cremona/22077' of git://trac.sagemath.org/sage into eclib
49aa039	trac #10256: update latest version of eclib from #22077

[sagetrac-git]

Changed commit from 724a205 to 49aa039

[chriswuthrich]

comment:21

With the latest changes to eclib, it looks good. Testing now.

[chriswuthrich]

comment:22

missed some doctests.

[JohnCremona]

comment:23

Replying to @categorie:

missed some doctests.

Dou mean that some are missing, or that they don't work?

[chriswuthrich]

comment:24

Sorry, I had to go, but now I am back. I meant: I missed updating the doctests in newforms.pyx with all the merging forth and back they got lost. Probably you are right and cleaning up could actually help. So one should take your new branch for #22077 and change the files in
in elliptic_curves:

• ell_modular_symbols
• ell_rational_field
• padic_lseries
• padics
  and in modular.pollack_stevens
• space
  There are also three misprints in newforms in libs.eclib which are better corrected in Upgrade eclib to version 20170104 #22077.

Shall I do it? Or have you almost done it already?

[chriswuthrich]

comment:25

John, I fear we are doing parallel work. Upload yours.

I did the changes here, but I ran into a very strange compilation error for newforms.pyx. Cython did not recognise "pair". I added an import, but then I got other compilation errors like

_sp = self.nfs.plus_modular_symbol(_r, 0, int(base_at_infinity))
sage/libs/eclib/newforms.pyx:347:46: Call with wrong number of arguments (expected 1, got 3)

Honestly I don't understand. So if yours works, I am happy to ditch what I have done here.

[JohnCremona]

comment:26

Sorry I totally messed up on the other ticket, and this one will have to wait until I sort that out, which will have to be Friday.

Correction (next morning): #22077 is fine and back to "needs review" -- see the remarks there. Nothing was messed up at all. I'll now finish cleaning up this one.

[JohnCremona]

Changed branch from u/wuthrich/ticket/10256 to u/cremona/10256new

[JohnCremona]

Changed commit from 49aa039 to none

[JohnCremona]

comment:28

OK so here's what I did. Starting from the new branch at #22077, I first used "git cherry-pick" to pick the 4 commits (6f517b3, 6301cc5, 6637227, e81588a) I had originally made here (which had been interspersed with eclib updates); only the first of those had some minor merge conflicts which I fixed. I then used rebase to sqush thos together into one commit ee879ee. Second, I did similarly with the 3 reviewer commits (dcacece, 724a205, e101d21), squashing into one (0db2a00). Lastly I added one more commit after testing revealed one doctest failing (49be7d8).

So I think that the 3 top commits on the new branch u/cremona/10256new contain all previous work while being based on u/cremona/22077new which is itself based on 7.5.rc1.

If there are additional changes needed here, that's fine as long as they are based on this (and don't require more changes to eclib itself...)

[chriswuthrich]

comment:29

Your branch doesn't exist, John. Mine still does not work.

[sagetrac-git]

Commit: 49be7d8

[sagetrac-git]

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

cb3fd05	#22077 update eclib to v20170104
ee879ee	work in progress on modular symbols
0db2a00	trac #10256: reviewer patches: first part, highlight wrong values, references
49be7d8	10256: fix one doctest

[JohnCremona]

comment:31

...it does help if you don't forget to actually push the branch to trac....it's there now

[chriswuthrich]

comment:32

All tests passed, the documentation, too. Thanks a lot.

[vbraun]

Changed branch from u/cremona/10256new to 49be7d8