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:

Status badges

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)

trac_12080_manin_constant.patch (6.3 KB) - added by wuthrich 10 years ago.
patch exported against 4.7.2

Download all attachments as: .zip

Change History (18)

comment:1 Changed 10 years ago by wuthrich

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)

comment:2 follow-up: Changed 10 years ago by cremona

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 wuthrich

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: Changed 10 years ago by wuthrich

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 cremona

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 wuthrich

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 was

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

Changed 10 years ago by wuthrich

patch exported against 4.7.2

comment:8 Changed 10 years ago by wuthrich

  • Authors set to wuthrich
  • 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: Changed 10 years ago by 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/

comment:10 in reply to: ↑ 9 ; follow-up: Changed 10 years ago by 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 should maybe include the paper as the basic reference in the docstring....

comment:11 in reply to: ↑ 10 Changed 10 years ago by cremona

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 was

  • Stopgaps set to todo

comment:13 in reply to: ↑ 4 Changed 10 years ago by was

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 was

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

comment:15 Changed 10 years ago by was

  • 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 jdemeyer

  • Authors changed from wuthrich to Chris Wuthrich
  • Reviewers set to William Stein
  • Stopgaps todo deleted

comment:17 Changed 10 years ago by jdemeyer

  • Merged in set to sage-5.0.beta12
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.