Improve minimal_quadratic_twist()

[JohnCremona]

We need a better implementation of minimal_quadratic_twist(), for example if the conductor is square-free then there is no need to do so much work as is currently done. Additionally, when j(E)=0 or 1728 the curve returned is a minimal quadratic twist, but not necessarily the minimal twist, and it would be a good thing for the documentation to make this clear.

CC: @categorie

Component: elliptic curves

Keywords: pari precision

Author: John Cremona

Branch/Commit: u/wuthrich/ticket/14060 @ 634ab95

Reviewer: Chris Wuthrich

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

[jdemeyer]

comment:2

Precision issue is #13163.

[JohnCremona]

comment:3

I see two possibilities here: either rename this function minimal_twist() and return (as it does now) a curve which may be a sextic or quartic twist when j=0 or j=1728; or keep this name and be more restrictive for those j-invariants so that the returned curve literally is a quadratic twist. A third possibility is to have both named functions, doing the same thing unless j=0 or j=1728.

I am CC-ing Chris W since the only place where this function is used (according to search_src()) is in sha_tate.py, line 540, and whatever we do must be compatible with what is needed there!

Of course this is in addition to adding one line to return self of self.conductor().is_squarefree().

[chriswuthrich]

comment:4

In sha_tate and in general for p-adic L-functions we can only twist by quadratic characters (well, I can only, maybe someone else knows more). So if changing the function one has to be careful to twist only by quadratic twists for these two cm curves. But if I remember correctly, we currently exclude cm curve (which we would not have to do in fact).

[JohnCremona]

comment:5

Looking more carefully at the code (I am not embarrassed that I wrote it but could not remember) I see that for j=0 and j=1728 the curve returned is always a quadratic twist. So the "additionally" part of the ticket's current description is incorrect.

If you agree with me on that (and a second opinion would be welcome!) then that just leaves the first part which I can easily implement by adding one line.

[chriswuthrich]

comment:6

We could have a minimal_twist function, too, if you wish, but I need minimal_quadratic_twist for modular symbols. If you can simplify the current function that is certainly a good thing to do.

I am not sure what changes you want to make, but if you do them, I should be able to look at them and review it quickly.

(from Besancon)

[JohnCremona]

Branch: u/cremona/ticket/14060

[JohnCremona]

Author: John Cremona

[JohnCremona]

Commit: dc07e49

[JohnCremona]

comment:8

I have implemented the small improvement (do nothing if the conductor is square-free) with a new doctest, and also expanded on the documentation in the exceptional cases j=0, 1728 to remove any confusion, with an example!

I decided not to implement a new minimal_twist() function since

EllipticCurve(j=E.j_invariant())

does that. But I could.

---

Last 10 new commits:

520cfca	Merge branch 'master' of git://github.com/sagemath/sage into build_system
38d22b7	Merge branch 'master' of ssh://trac.sagemath.org:2222/sage into build_system
139dcf5	Merge branch 'build_system' of git://github.com/sagemath/sage into build_system
17e17dd	Merge branch 'master' of ssh://trac.sagemath.org:2222/sage into build_system
b06bd7c	Merge branch 'master' of git://github.com/sagemath/sage
858bb95	Merge branch 'master' of git://github.com/sagemath/sage
0523e0d	Merge branch 'master' of git://github.com/sagemath/sage
88ffd55	Merge branch 'master' of git://github.com/sagemath/sage
68d0a5d	Merge branch 'master' of git://github.com/sagemath/sage
057b3cd	Merge branch 'master' of git://github.com/sagemath/sage

[JohnCremona]

comment:9

There are lots of commits listed above, none of which is mine. The relevant one, which you can see by clicking on the branch name, is dc07e494c5230bef1dd7e1a0df351194a03acb36 .

[fchapoton]

comment:10

the note field is only indented by 3 spaces, there should be 4

[chriswuthrich]

Changed branch from u/cremona/ticket/14060 to u/wuthrich/ticket/14060

[chriswuthrich]

Changed commit from dc07e49 to 634ab95

[chriswuthrich]

comment:11

I rebased it to the 6.1.beta6 in the hope that the extra merge commits above do not show anymore (even if they are harmless.

Since all test then pass for me, I give it a positive review.

---

New commits:

634ab95

trac #14060: improvement to minimal_quadratic_twist

[chriswuthrich]

Reviewer: Chris Wuthrich

[chriswuthrich]

comment:12

By the way, I believe there are 4 spaces before the ..note. No ? At least the html output seems ok.

[JohnCremona]

comment:13

Replying to @categorie:

By the way, I believe there are 4 spaces before the ..note. No ? At least the html output seems ok.

I don't know, I thought that not warnings were produced when building the docs but have forgotten.

Thanks for the review!