Opened 12 years ago

Closed 12 years ago

# elliptic curve canonical height bug for non-minimal models

Reported by: Owned by: cremona cremona major sage-4.3.4 elliptic curves canonical height sage-4.3.4.alpha0 John Cremona Chris Wuthrich N/A

### Description

For canonical heights of points on elliptic curves defined over QQ, we call the pari function ellheight(), BUT that function only gives the correct value for global minimal models! (At primes where the model is not minimal the local component is wrong).

Here is an example to show this:

```sage: E = EllipticCurve([-5580472329446114952805505804593498080000,
....:           -157339733785368110382973689903536054787700497223306368000000])
sage: P3=E([204885147732879546487576840131729064308289385547094673627174585676211859152978311600/23625501907057948132262217188983681204856907657753178415430361,92736254270288706000052529616433462503893110244786566575440613880920567197984949809570153263207165494624214991751142500000000/114834283590033957142081201488956527887145361353994063932282392800295014255070987824900081891])
sage: P3.height()
157.086024926474
sage: 4*(P3.height())-(2*P3).height() # should == 0
-1.38629436111989
```

while on the minimal model:

```sage: Emin = E.minimal_model()
sage: urst = E.isomorphism_to(Emin)
sage: 4*urst(P3).height()-urst(2*P3).height()
0.000000000000000
```

The cure is to compute the minimal model and transfer the point there before computing the height, as illustrated above. (Of course, pari could do that too, but this behaviour has been tolerated by pari users for many years!)

Applies to 4.3.3

### comment:1 Changed 12 years ago by cremona

• Authors set to John Cremona
• Milestone set to sage-4.3.4
• Status changed from new to needs_review

The patch implements the cure. I did not bother putting in a test for minimality of the original model, since all that does is to compute the minimal model anyway and compare! Also, the minimal model is cached, so that would only be done once. We do not cache the isomorphism to the minimal model, but that is cheap to compute.

### comment:2 Changed 12 years ago by wuthrich

• Reviewers set to Chris Wuthrich
• Status changed from needs_review to positive_review

Looks fine to me. All tests pass.

### comment:3 Changed 12 years ago by mvngu

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