Opened 10 years ago
Closed 10 years ago
#12080 closed defect (fixed)
manin constant
Reported by: | wuthrich | Owned by: | cremona |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | elliptic curves | Keywords: | manin constant, isogeny |
Cc: | was | Merged in: | sage-5.0.beta12 |
Authors: | Chris Wuthrich | Reviewers: | William Stein |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
My definition of the Manin constant of an elliptic curve E/Q is the rational number c such that
phi(omega) = c * f dq/q
where
- phi is the modular parametrisation X_0(N) -> E of minimal degre
- omega is the Neron differential on E
- f is the normalised newform
- q is q=exp(2pi i tau) as usual
With this definition I get a different answer than sage. For instance for 11a2, I get 1 not 5.
Either one has to change the implementation or one has to add to the documentation the definition of what is computed.
The current implementation (from #5138 ) computes the minimal degree of an isogeny from E to the X_0-optimal curve E_0 and multiplies the manin constant of E_0 by this degree. Instead, with my definition, we have to multiply with the number c' where psi*(omega) = c' * omega_0 with psi the isogeny E_0 -> E of minimal degree and omega_0 the Neron differential of E_0. This c' is a divisor of the degree of psi, but on many occasions it is 1.
Attachments (1)
Change History (18)
comment:1 Changed 10 years ago by
comment:2 follow-up: ↓ 3 Changed 10 years ago by
Interesting. All Sage does is call sympow (Mark Watkins's program). Magma gives the same output as Sage (not surprising since it also uses MW's code). Have you tried asking MW for his rationale?
comment:3 in reply to: ↑ 2 Changed 10 years ago by
Replying to cremona:
Interesting. All Sage does is call sympow (Mark Watkins's program).
Does it ? I see these lines in the code....
label = self.cremona_label() optimal = self.optimal_curve() if optimal == self: return Integer(1) L, v = self._shortest_paths() i = L.index(optimal) return v[i]
and shortest_path
is what you think it is.
comment:4 follow-up: ↓ 13 Changed 10 years ago by
magma, by the way, gets the correct answer (meaning the one consistent with my definition above).
> E := EllipticCurve(CremonaDatabase(), "11a2"); > ManinConstant(E); 1 > E := EllipticCurve(CremonaDatabase(), "11a3"); > ManinConstant(E); 5 > E := EllipticCurve(CremonaDatabase(), "11a1"); > ManinConstant(E); 1
comment:5 Changed 10 years ago by
Now I am confused. First, the code I see for E.modular_degree() in 4.7.2 looks like:
try: return self.__modular_degree except AttributeError: if algorithm == 'sympow': from sage.lfunctions.all import sympow m = sympow.modular_degree(self) elif algorithm == 'magma': from sage.interfaces.all import magma m = rings.Integer(magma(self).ModularDegree()) else: raise ValueError, "unknown algorithm %s"%algorithm self.__modular_degree = m return m
but I admit that I did not look to see what sympow.modular_degree() does.
Secondly, in Magma V2.17-9 I get
> ModularDegree(EllipticCurve("11a2")); 5
comment:6 Changed 10 years ago by
I am talking about the MANIN CONSTANT not the MODULAR DEGREE !
The Manin constant as I define it in the description is not the degree, e.g. for 11a2 we have an isogeny X_0(11) = 11a1 -> 11a2 of degree 5, but the pullback of the Neron differential is the Neron differential as the map is etale with kernel Z/5Z.
The above code is in manin_constant
and was added in #5138.
comment:7 Changed 10 years ago by
Chris, you're right: the code in sage should change, and docs be improved. I was sloppy.
comment:8 Changed 10 years ago by
- Status changed from new to needs_review
Here is a patch for the problem. Now, the algorithm is based on comparing the period lattices for the elliptic curve and the optimal curve in the isogeny class.
I checked this implementation against the one in magma. for conductors below 1000, there are two exceptions, 27a4 and 80b4. I included them as doctests. I believe that this patch returns the correct value and magma has a small bug in its ManinConstant? function.
So the reviewer is asked to compute by hand the Manin constant for these two curves and confirm that the returned value is indeed correct. Then I will send a bug report to magma.
Furthermore, a precise question to John : Is it true (as the algorithm here assumes) that you have checked that the Manin constant is 1 for all optimal curves in your table ?
comment:9 follow-up: ↓ 10 Changed 10 years ago by
Wuthrich: the appendix by Cremona to this paper is about him verifying that the Manin constant is 1: http://wstein.org/papers/ars-manin/
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 11 Changed 10 years ago by
Replying to was:
Wuthrich: the appendix by Cremona to this paper is about him verifying that the Manin constant is 1: http://wstein.org/papers/ars-manin/
Yes, indeed. I was referring to this. But at the time the table was smaller, so I jsut wanted to check that the assumption underlying the algorithm is known to hold for all curves in the tale (now and in the future).
I should maybe include the paper as the basic reference in the docstring....
comment:11 in reply to: ↑ 10 Changed 10 years ago by
Replying to wuthrich:
Replying to was:
Wuthrich: the appendix by Cremona to this paper is about him verifying that the Manin constant is 1: http://wstein.org/papers/ars-manin/
Yes, indeed. I was referring to this. But at the time the table was smaller, so I jsut wanted to check that the assumption underlying the algorithm is known to hold for all curves in the tale (now and in the future).
I will look into this, as I have been meaning to anyway. There was some extra work to do when I wrote that appendix, and so there may also be some to do to verify that c=1 for the extended tables. Some of this is automatically done on the fly, but not all. It *is* true that when there is only one curve in the isogeny class that c=1 -- this is not as trivial a statement as it appears, but it amounts to saying that the optimal curve is minimal, and I do check that on the fly.
I should maybe include the paper as the basic reference in the docstring....
comment:12 Changed 10 years ago by
- Stopgaps set to todo
comment:13 in reply to: ↑ 4 Changed 10 years ago by
Replying to wuthrich:
magma, by the way, gets the correct answer (meaning the one consistent with my definition above).
I finally wrote the obvious program that one should write when ref'ing this patch:
wstein@boxen:/tmp/wstein/sage-5.0.b9$ more 12080.sage for E in cremona_curves([1..10000]): N = E.conductor() if N%100==0: print N c = E.manin_constant() d = magma(E).ManinConstant() if c != d: print E.cremona_label(), c, d
When running it up to 300, I get two differences from Magma:
27a4 3 1 80b4 2 1 nothing else up to 300
It could very well be that Magma is wrong... but as part of this patch, could somebody get to the bottom of this?
comment:14 Changed 10 years ago by
I've asked Mark Watkins to look at this, since he wrote ManinConstant? in Magma....
comment:15 Changed 10 years ago by
- Status changed from needs_review to positive_review
Mark Watkins says: "Sage is definitely correct. Sympow code I wrote agrees with Sage here. Magma has a bug."
So... positive review. (Also, I checked up to 1200 and Sage and Magma agree in the rest of the cases.)
Incidentally, Magma's code is way faster:
sage: E = list(cremona_curves([1200]))[0] sage: E Elliptic Curve defined by y^2 = x^3 - x^2 + 17*x - 38 over Rational Field sage: time E.manin_constant() 1 Time: CPU 0.71 s, Wall: 0.82 s sage: F = magma(E) sage: time F.ManinConstant() 1 Time: CPU 0.00 s, Wall: 0.06 s
However, according to Mark Watkins, Magma is fast because it assumes every imaginable conjecture in that implementation (and doesn't actually compute any periods, etc.)
comment:16 Changed 10 years ago by
- Reviewers set to William Stein
- Stopgaps todo deleted
comment:17 Changed 10 years ago by
- Merged in set to sage-5.0.beta12
- Resolution set to fixed
- Status changed from positive_review to closed
The following function, which I wrote for another thing, computes the valuation of c' as above. So this may be useful if it is decided that the implementation rather than the documentation of
manin_constant
needs changing.