Skip to content
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

manin constant #12080

Closed
chriswuthrich opened this issue Nov 24, 2011 · 21 comments
Closed

manin constant #12080

chriswuthrich opened this issue Nov 24, 2011 · 21 comments

Comments

@chriswuthrich
Copy link
Contributor

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.

CC: @williamstein

Component: elliptic curves

Keywords: manin constant, isogeny

Author: Chris Wuthrich

Reviewer: William Stein

Merged: sage-5.0.beta12

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

@chriswuthrich
Copy link
Contributor Author

comment:1

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.

def neron_scaling(phi,v):
    r"""
    Given an isogeny `\phi` between elliptic curves over a number field and a place `v`,
    this gives back the valuation at `v` of the constant c such that
    `\phi*(\omega') = c \cdot \omega`
    where `\omega` and `\omega'` are Neron differentials.
    
    INPUT:
    
    - ``phi`` - an isogeny between elliptic curves over a number field
    
    - ``v`` - a finite place of this field
    
    OUTPUT: an integer
    
    Note: This only makes sense if ``v`` devides the degree of ``phi``.
    
    EXAMPLES::

        sage: E = EllipticCurve("20a1")
        sage: P= E.lift_x(0)
        sage: P.order()
        3
        sage: neron_scaling(phihat,3)
        1
        sage: neron_scaling(phi,3)
        0
     
        sage: E = EllipticCurve("11a1")
        sage: E = E.change_weierstrass_model([5,1,0,0])
        sage: E2 = EllipticCurve("11a2").change_weierstrass_model([1/25,0,1,0])
        sage: P = E.torsion_points()[2]
        sage: phi = E.isogeny(P, codomain=E2)
        sage: neron_scaling(phi,5)
        0
        sage: neron_scaling(phi.dual(),5)
        1
            
    """
    E = phi.domain()
    E2 = phi.codomain()
    Emin = E.local_data(v).minimal_model()
    E2min = E2.local_data(v).minimal_model()
    u = E.isomorphism_to(Emin).u
    u2 = E2.isomorphism_to(E2min).u
    a1 = phi.formal()[1]
    return valuation(a1,v) - valuation(u,v) + valuation(u2,v)

@JohnCremona
Copy link
Member

comment:2

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?

@chriswuthrich
Copy link
Contributor Author

comment:3

Replying to @JohnCremona:

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.

@chriswuthrich
Copy link
Contributor Author

comment:4

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

@JohnCremona
Copy link
Member

comment:5

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

@chriswuthrich
Copy link
Contributor Author

comment:6

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.

@williamstein
Copy link
Contributor

comment:7

Chris, you're right: the code in sage should change, and docs be improved. I was sloppy.

@chriswuthrich
Copy link
Contributor Author

patch exported against 4.7.2

@chriswuthrich
Copy link
Contributor Author

comment:8

Attachment: trac_12080_manin_constant.patch.gz

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 ?

@chriswuthrich
Copy link
Contributor Author

Author: wuthrich

@williamstein
Copy link
Contributor

comment:9

Wuthrich: the appendix by Cremona to this paper is about him verifying that the Manin constant is 1:
http://wstein.org/papers/ars-manin/

@chriswuthrich
Copy link
Contributor Author

comment:10

Replying to @williamstein:

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

@JohnCremona
Copy link
Member

comment:11

Replying to @categorie:

Replying to @williamstein:

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

@williamstein
Copy link
Contributor

Stopgaps: todo

@williamstein
Copy link
Contributor

comment:13

Replying to @categorie:

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?

@williamstein
Copy link
Contributor

comment:14

I've asked Mark Watkins to look at this, since he wrote ManinConstant in Magma....

@williamstein
Copy link
Contributor

comment:15

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

@jdemeyer
Copy link

Reviewer: William Stein

@jdemeyer
Copy link

Changed stopgaps from todo to none

@jdemeyer
Copy link

Changed author from wuthrich to Chris Wuthrich

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2012

Merged: sage-5.0.beta12

@jdemeyer jdemeyer closed this as completed Apr 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants