Opened 9 years ago
Closed 9 years ago
#13953 closed defect (fixed)
(non)archimedian_local_height of a torsion points always gives 0
Reported by: | pbruin | Owned by: | cremona |
---|---|---|---|
Priority: | major | Milestone: | sage-5.10 |
Component: | elliptic curves | Keywords: | local heights |
Cc: | Merged in: | sage-5.10.beta4 | |
Authors: | Peter Bruin | Reviewers: | Chris Wuthrich |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12509 | Stopgaps: |
Description (last modified by )
For torsion points of elliptic curves over number fields, nonarchimedian_local_height incorrectly returns 0, and archimedian_local_height raises an error:
sage: K.<i> = QuadraticField(-1) sage: E = EllipticCurve([0, 0, 0, K(1), 0]) sage: P = E(i, 0) sage: P.nonarchimedian_local_height() 0 sage: P.archimedian_local_height() NameError: global name 'QQ' is not defined
The correct behaviour (for the normalisation used in Sage) is
sage: P.nonarchimedian_local_height() -1/2*log(2) sage: P.archimedian_local_height() 0.346573590279973
Note: the same would happen for rational points on elliptic curves over Q, but computing local heights over Q is broken (see #13951).
Apply: trac13953-local_heights_torsion.patch
Attachments (1)
Change History (12)
comment:1 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 9 years ago by
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 9 years ago by
Replying to cremona:
(1) is this patch based on #12509?
No, but I have another branch where I made the same patch based on #12509; would it be better to upload that one?
(2) is your fix just to remove the wrong quick exit for torsion points (as appears)?
Yes, the rest of the patch just adds a doctest.
comment:4 in reply to: ↑ 3 Changed 9 years ago by
comment:5 Changed 9 years ago by
- Dependencies set to #12509
comment:6 Changed 9 years ago by
- Reviewers set to Chris Wuthrich
- Status changed from needs_review to positive_review
The patch applies cleanly to 5.9 after #12509. The tests pass and it does what it should do.
comment:7 Changed 9 years ago by
- Status changed from positive_review to needs_work
comment:8 Changed 9 years ago by
- Status changed from needs_work to needs_review
comment:9 Changed 9 years ago by
- Status changed from needs_review to positive_review
Thanks for rebasing it. (Sorry for not commenting, the browser in my office decided to log me out permanently so I could not post a comment on what needs to be done).
I tested it again and it is again all fine.
comment:10 Changed 9 years ago by
Thanks! And no problem, it was clear what had to be done.
comment:11 Changed 9 years ago by
- Merged in set to sage-5.10.beta4
- Resolution set to fixed
- Status changed from positive_review to closed
2 quick questions: (1) is this patch based on #12509? (2) is your fix just to remove the wrong quick exit for torsion points (as appears)?