Bug in descend_to method for elliptic curves #16456

JohnCremona · 2014-06-07T17:06:54Z

The function descend_to which was implemented in #9384 is incorrect. The twisting parameter d in L* is only defined modulo (L*)^2 and one has to determine whether there is a representative which lies in K*, which the implementation does not do, so some "positive" results are missed. Here is an example of this.

sage: K.<a> = NumberField(x^3-2)
sage: E = EllipticCurve('11a1').quadratic_twist(2)
sage: EK = E.change_ring(K)
sage: EK2 = EK.change_weierstrass_model((a,a,a,a+1))
sage: EK2.descend_to(QQ)
<None>

The heart of the problem being solved here is to determine whether (given that j(E) is in K of course) the twisting parameter in L*/(L*)^2 is in the image of K*/(K*)^2 or not, and the implementation ignores this. (For j=0 or 1728 the principle is the same with squares replaced by 6th or 4th powers respectively.)

I can fix this for number fields (one can restrict from K*/(K*)^2 to a finite K(S,2) for an easily determined set S of primes) or for finite fields, but it may not be possible to have this implemented for arbitrary fields, which will cause a problem with the patching.

A second and independent bug is reported by Warren Moore:

sage: k.<i> = QuadraticField(-1)
sage: E = EllipticCurve(k,[0,0,0,1,0])
sage: E.descend_to(QQ) == None
True

This caused by a "naked Except" clause plus a call to f.preimage() for a map f which has no attribute/method "preimage".

Depends on #16708

Component: elliptic curves

Keywords: elliptic curve base change

Author: John Cremona

Branch/Commit: e0a13b3

Reviewer: Peter Bruin

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

The text was updated successfully, but these errors were encountered:

JohnCremona · 2014-06-13T13:54:39Z

Branch: u/cremona/bug_in_descend_to_method_for_elliptic_curves

JohnCremona · 2014-06-13T13:58:59Z

Commit: 9b0a5d9

JohnCremona · 2014-06-13T13:58:59Z

Author: John Cremona

JohnCremona · 2014-06-13T13:58:59Z

comment:5

Ready for review. Note that there are two separate things here (which perhaps should have been in separate tickets):

Up till the last commit the changes are in in number fields (including QQ), not touching elliptic curves, implementing the function descend_mod_power for number field elements and enhancing the selmer_group with selmer_group_iterator for ease of use. (Also correcting the documentation for selmer_group).

The last commit does what the ticket wanted, namely to re-implement the descend_to function for elliptic curves. Apart from trivial cases this will only work over number fields. It would work over any other field where descend_mod_power could be implemented such as finite fields, but note that in characteristics 2 and 3 the function would need to have a completely different implementation.

New commits:

`04c1c45`	`Enhanced number field selmer group capabilities by adding an iterator`
`9b0a5d9`	`#16456: fix bugs in elliptic curve descend_to function`

sagetrac-git · 2014-07-14T13:31:32Z

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

`d5c4ba7`	`Merge branch 'develop' into t/16456/bug_in_descend_to_method_for_elliptic_curves`

sagetrac-git · 2014-07-14T13:31:32Z

Changed commit from 9b0a5d9 to d5c4ba7

JohnCremona · 2014-07-21T13:13:41Z

New commits:

`12c1b73`	`minor addition to isogeny_small_degree for sporadic primes over number fields`
`1c3ddb9`	`Merge branch 'develop' into isogs`
`3db6c28`	`Merge branch 'develop' into isogs`

JohnCremona · 2014-07-21T13:13:41Z

Changed commit from d5c4ba7 to 3db6c28

JohnCremona · 2014-07-21T13:13:41Z

Changed branch from u/cremona/bug_in_descend_to_method_for_elliptic_curves to u/cremona/16456

pjbruin · 2014-07-22T09:45:16Z

Reviewer: Peter Bruin

pjbruin · 2014-07-22T09:45:16Z

comment:8

I remember looking at the Selmer group code some time ago (and again just now) and getting the impression that the group K(S, m) returned by selmer_group(S, m) is the one fitting in a natural exact sequence (warning: unicode experiment ahead)

1 ⟶ O_K,S^×/O_K,S^×m ⟶ K(S, m) ⟶ Cl_K,S[m] ⟶ 0.

(I.e., I think K(S, m) should be canonically isomorphic to the flat cohomology group H¹(O_K,S, μ_m). This can be replaced by étale cohomology if S contains all places dividing m, and in that case I also think that the above H¹, and K(S, m), should be isomorphic to H¹(Gal(K^S/K), μ_m) with K^S the maximal extension of K that is unramified outside S.)

Is the above exact sequence correct, and should it perhaps be mentioned in the documentation? And is there a comparable exact sequence for the "true" Selmer group consisting of the elements giving unramified extensions of K?

JohnCremona · 2014-07-22T10:31:26Z

comment:9

See #16496 where you will find that exact sequence!

One thing which makes this sequence easier to deal with than one might expect is that the group in the middle is a direct product of the other two rather than some more complicated extension.

pjbruin · 2014-07-22T16:09:41Z

comment:10

Replying to @JohnCremona:

See #16496 where you will find that exact sequence!

Right! I am sure I have seen that ticket before...

One thing which makes this sequence easier to deal with than one might expect is that the group in the middle is a direct product of the other two rather than some more complicated extension.

That sounded surprising to me at first, but I tried to verify it and realised I had actually also done something very much like that before.

pjbruin · 2014-07-22T16:17:46Z

Changed commit from 3db6c28 to 65d3404

pjbruin · 2014-07-22T16:17:46Z

comment:11

Here is a reviewer patch mostly consisting of documentation/formatting changes. If you agree with my changes, you can set it to positive review.

pjbruin · 2014-07-22T16:17:46Z

Changed branch from u/cremona/16456 to u/pbruin/16456-descend_to_bug

pjbruin · 2014-07-23T20:13:23Z

comment:12

While trying to find a clever cohomological argument for the splitting of the exact sequence from comment:8, I did not succeed but did find a bug in selmer_group(); see #16708.

sagetrac-git · 2014-07-23T20:24:45Z

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

`bed05c7`	`Trac 16456: corrected doctest changed by #16708`

sagetrac-git · 2014-07-23T20:24:45Z

Changed commit from 65d3404 to bed05c7

pjbruin · 2014-07-23T20:25:16Z

Dependencies: #16708

sagetrac-git · 2014-07-23T20:32:11Z

Changed commit from bed05c7 to 65d3404

sagetrac-git · 2014-07-23T20:32:11Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

pjbruin · 2014-07-23T20:35:10Z

comment:16

Actually both the original answer and my "corrected" one in the doctest changed by commit bed05c5 are wrong; if I'm not mistaken, the correct answer is

sage: K = QuadraticField(-5)
sage: list(K.selmer_group_iterator((), 4))
[1, 4, -1, -4]

JohnCremona · 2014-07-23T20:55:38Z

comment:17

Replying to @pjbruin:

Actually both the original answer and my "corrected" one in the doctest changed by commit bed05c5 are wrong; if I'm not mistaken, the correct answer is
sage: K = QuadraticField(-5)
sage: list(K.selmer_group_iterator((), 4))
[1, 4, -1, -4]

That is certainly correct. I will look at this and #16708 tomorrow.

JohnCremona · 2014-07-24T11:43:50Z

comment:18

Between here and #16708 a doctest needed changing as I think Peter pointed out somewhere:

            sage: K.<a> = QuadraticField(-5)
            sage: list(K.selmer_group_iterator((), 4))
            [1, 2, 4, 8, -1, -2, -4, -8]

should be

           sage: K.<a> = QuadraticField(-5)
           sage: list(K.selmer_group_iterator((), 4))
           [1, 4, 16, 64, -1, -4, -16, -64]

which I am changing and will commit & push.

JohnCremona · 2014-07-24T11:45:05Z

Changed commit from 65d3404 to e6ccee2

JohnCremona · 2014-07-24T11:45:05Z

Changed branch from u/pbruin/16456-descend_to_bug to u/cremona/16456-descend_to_bug

JohnCremona · 2014-07-24T11:45:05Z

New commits:

`8474e25`	`Trac 16708: fix computation of Selmer groups of number fields`
`c584016`	`Merge remote-tracking branch 'trac/u/pbruin/16456-descend_to_bug' into 16708`
`e6ccee2`	`#16456: fix one doctest after #16708`

JohnCremona · 2014-07-24T11:47:55Z

comment:20

Peter, thanks for your tidying up of (and improving) the documentation. I just fixed that one doctest which I think you already noticed was incorrect because of the bug fixed now at #16708. That means that the branch name has gone back to mine (u/cremona/...), and I have set it back to "needs review". Since I like your reviewer's changes, if you agree wit hthis last one then we can set it to "positive review".

pjbruin · 2014-07-24T11:56:32Z

comment:21

Replying to @JohnCremona:

Between here and #16708 a doctest needed changing as I think Peter pointed out somewhere:

            sage: K.<a> = QuadraticField(-5)
            sage: list(K.selmer_group_iterator((), 4))
            [1, 2, 4, 8, -1, -2, -4, -8]

should be

           sage: K.<a> = QuadraticField(-5)
           sage: list(K.selmer_group_iterator((), 4))
           [1, 4, 16, 64, -1, -4, -16, -64]

which I am changing and will commit & push.

That second answer was actually the wrong "correction" I pushed and then retracted. It should be [1, 4, -1, -4] (as in comment:17); in fact 16 is a fourth power, hence represents the trivial element of the Selmer group. The Selmer group is isomorphic to C₂ x C₂ (since both the units modulo 4-th powers and the 4-torsion in the class group are cyclic of order 2.)

I think the problem is the line

f = lambda o: m if o is Infinity else o.gcd(m)

where you will somehow need to take the orders of elements of the class group into account.

JohnCremona · 2014-07-24T12:31:03Z

comment:22

Sorry about that. I'll work out how to correct it....

sagetrac-git · 2014-07-24T13:27:09Z

Changed commit from e6ccee2 to e0a13b3

sagetrac-git · 2014-07-24T13:27:09Z

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

`e0a13b3`	`#16456: fix bug in orders of Selmer group elements`

JohnCremona · 2014-07-24T13:32:34Z

comment:24

...done. I added an optional parameter "orders" to the selmer_group method, default False gives the old behavious, if True also outputs a list of the orders of the generators, which are all equal to m when m is prime, but not necesarily otherwise. Added doctests for this. Now the iterator uses this instead of the old formula, which was wrong. I thought that this was simpler than trying to recover the orders from the list of generators, since it is information we have to hand when constructing the list of generators.

For consistency I did the same for the special versions over QQ, though the bug did not apply here. At the same time I noticed that over QQ when -1 is a generator it was put last while over number fields the units are always first, so I changed that (and the associated doctests) too.

Note that this change will be superceded after #16496 when there is a proper class for these Selmer groups. And also that the correctness of the current selmer_group function relies on the direct product fact alluded to in earlier comments!

pjbruin · 2014-07-24T15:14:54Z

comment:25

OK, everything looks good to me now!

vbraun · 2014-07-25T05:05:41Z

Changed branch from u/cremona/16456-descend_to_bug to e0a13b3

JohnCremona added this to the sage-6.3 milestone Jun 7, 2014

JohnCremona added c: elliptic curves labels Jun 7, 2014

This comment has been minimized.

Sign in to view

JohnCremona self-assigned this Jun 7, 2014

This comment has been minimized.

Sign in to view

JohnCremona added the s: needs review label Jun 13, 2014

pjbruin added s: needs work and removed s: needs review labels Jul 23, 2014

JohnCremona added s: needs review and removed s: needs work labels Jul 24, 2014

pjbruin added s: needs work and removed s: needs review labels Jul 24, 2014

JohnCremona added s: needs review and removed s: needs work labels Jul 24, 2014

pjbruin added s: positive review and removed s: needs review labels Jul 24, 2014

vbraun removed the s: positive review label Jul 25, 2014

vbraun closed this as completed in 6af23b7 Jul 25, 2014

Bug in descend_to method for elliptic curves #16456

Bug in descend_to method for elliptic curves #16456

Comments

JohnCremona commented Jun 7, 2014

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

JohnCremona commented Jun 13, 2014

JohnCremona commented Jun 13, 2014

JohnCremona commented Jun 13, 2014

JohnCremona commented Jun 13, 2014

sagetrac-git mannequin commented Jul 14, 2014

sagetrac-git mannequin commented Jul 14, 2014

JohnCremona commented Jul 21, 2014

JohnCremona commented Jul 21, 2014

JohnCremona commented Jul 21, 2014

pjbruin commented Jul 22, 2014

pjbruin commented Jul 22, 2014

JohnCremona commented Jul 22, 2014

pjbruin commented Jul 22, 2014

pjbruin commented Jul 22, 2014

pjbruin commented Jul 22, 2014

pjbruin commented Jul 22, 2014

pjbruin commented Jul 23, 2014

sagetrac-git mannequin commented Jul 23, 2014

sagetrac-git mannequin commented Jul 23, 2014

pjbruin commented Jul 23, 2014

sagetrac-git mannequin commented Jul 23, 2014

sagetrac-git mannequin commented Jul 23, 2014

pjbruin commented Jul 23, 2014

JohnCremona commented Jul 23, 2014

JohnCremona commented Jul 24, 2014

JohnCremona commented Jul 24, 2014

JohnCremona commented Jul 24, 2014

JohnCremona commented Jul 24, 2014

JohnCremona commented Jul 24, 2014

pjbruin commented Jul 24, 2014

JohnCremona commented Jul 24, 2014

sagetrac-git mannequin commented Jul 24, 2014

sagetrac-git mannequin commented Jul 24, 2014

JohnCremona commented Jul 24, 2014

pjbruin commented Jul 24, 2014

vbraun commented Jul 25, 2014