-
-
Notifications
You must be signed in to change notification settings - Fork 419
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
bug in modular symbols for elliptic curves #10236
Comments
comment:1
One guess might be that I misinterpreted what the output of eclib is. I assumed that in line 553 in ell_modular_symbol.py that the output of
meant that 2 pi i \int_{r}^{0} f(z) dz = m(r) * Omega |
comment:2
The second bug is independent of the first and I found the erro in the original implementation. I will soon post a patch that covers this part of the problem. |
Attachment: trac_10236_1.patch.gz This first patch solves the second issue. Exported 4.6 |
comment:3
I found all the problems now... they are all due to ME. Sorry ! I will post a patch shortly. |
comment:4
On the way I found what the second patch in #9476 (changeset 14823) did. I have to revert that. It is true that eclib has now negative spaces, but the call function for negatives is not implemented in newforms.pyx. So as it was until now, E.modular_symbol(use_eclib=True, sign=-1)(r) will compute the + modular symbol. I am sorry that I did not take a look at #9476. Hence I will also revert the part of #9247 that made eclib to be chosen by default in p-adic L-functions. I will open a separate ticket for changing this afterwards. |
Attachment: trac_10236_2.patch.gz Solves the remaining problems. Exported 4.6. This patch has to be applied after the first one. |
Author: Chris Wuthrich |
comment:5
This second patch, applied after the first one, will solve the both issues in the ticket description and revert back some errors introduced earlier. The problem was a "/2" in a formula. I have absolutely no idea why I put that there. To check that the formula is now correct, I did the following::
which gladly did not yield any output. So these patches are ready for review. |
comment:6
The follow up about the negative modular symbols is at #10256. |
comment:7
I guess that at least some of this is my fault, for not doing everything all at once at #9476. I hope to get time to review this before too long... |
comment:8
sorry, there are docstrings to change in ell_rational_field as well. I will do that later today. |
exported 4.6 to be applied after the two others |
Attachment: trac_10236_3.patch.gz Attachment: trac_10236_4.patch.gz to be applied after the other three patches |
comment:9
I hope now I have found them all. |
comment:10
Patches look good (note: the last two are just fixing old wrong doctest output!) and apply fine to 4.6.1.alpha1. I am testing now. |
Reviewer: John Cremona |
comment:11
Replying to @JohnCremona:
All pass! |
comment:12
Milestone is 4.6.2 (if you want this merged in 4.6.1, change the milestone accordingly) |
Merged: sage-4.6.1.alpha3 |
The following two computations should yield the same answer. First with the modular symbols in sage
and then using ec_lib :
That the actual value of [1/7] must be 7/10 is illustrated be the following
The fact that both values at 0 are equal show that it is unlikely that this is a problem with the scaling of the modular symbols.
Here is another bug. Maybe the same, maybe different. This one looks like being in scaling. But I am puzzled, because this example was used originally in the design of the scaling function.
It should in fact be [1/7]+ = 1/2.
This was originally reported by Andrew Ohana.
CC: @JohnCremona @williamstein
Component: elliptic curves
Keywords: modular symbols
Author: Chris Wuthrich
Reviewer: John Cremona
Merged: sage-4.6.1.alpha3
Issue created by migration from https://trac.sagemath.org/ticket/10236
The text was updated successfully, but these errors were encountered: