sha().an() assumes E is minimal.

[sagetrac-weigandt]

I noticed the following problem with sha().an()

sage: E=EllipticCurve([1215*1216,0]) # non-minimal model
sage: E.sha().an()
6.00000000000000
sage: E.minimal_model().sha().an()
1.00000000000000

It looks like sha().an() assumes that E is minimal. The extra factor of 6 seems to be coming from the real period.

sage: E.period_lattice().omega()
0.106360349280819
sage: E.minimal_model().period_lattice().omega()
0.638162095684913

It's probably unfair to call this a bug, but it could definitely lead people astray.

CC: @williamstein @rlmill @categorie

Component: elliptic curves

Keywords: sha, real period

Author: John Cremona

Reviewer: Aly Deines

Merged: sage-4.6.1.alpha1

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

[JohnCremona]

Changed upstream from Reported upstream. Developers acknowledge bug. to none

[JohnCremona]

comment:1

I found the problem here. an() uses the minimal model when it can do the job itself, which is when the rank is at most 1, but when the rank is at least 2 it passes the work to an_numerical(), and that did not use the minimal model.

I am preparing a patch which fixes this. And as I suspect that other related functions also need the minimal model (notably the p-adic one) I am now caching the minimal model and changing lots of occurrences of self.E to self.Emin.

Watch this space...

[JohnCremona]

Applies to 4.6.rc0

[JohnCremona]

Author: John Cremona

[JohnCremona]

comment:2

Attachment: trac_10096-sha.patch.gz

The patch fixes this. the minimal model is computed on construction and used in place of the original (when the latter is not minimal). New doctests show that the reported problem has gone away.

It would be helpful if someone could confirm that this is the correct thing to do also for the an_padic() function. The doctests still pass, but I sust[ect that they are all with minimal curves anyway. I was assuming (a) that the value of an_padic() was isomorphism-invariant, and (2) that using a minimal model would be at least as fast, possibly faster, than using a non-minimal model, even if both give the correct answer!

[jdemeyer]

Reviewer: Aly Deines

[jdemeyer]

Merged: sage-4.6.1.alpha1

[jdemeyer]

Changed reviewer from Aly Deines to Alyson Deines

[jdemeyer]

Changed reviewer from Alyson Deines to Aly Deines