Opened 12 years ago
Closed 12 years ago
#9372 closed enhancement (fixed)
implement regulator function for elliptic curves over number fields
Reported by: | cremona | Owned by: | cremona |
---|---|---|---|
Priority: | major | Milestone: | sage-4.5.2 |
Component: | elliptic curves | Keywords: | |
Cc: | robertwb | Merged in: | sage-4.5.2.alpha0 |
Authors: | John Cremona | Reviewers: | David Loeffler, Robert Bradshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Now that we have canonical heights over number fields, the regulator_of_points code can be moved up from ell_rational_field to ell_number_field.
Attachments (2)
Change History (11)
comment:1 Changed 12 years ago by
- Component changed from algebra to elliptic curves
- Owner changed from AlexGhitza to cremona
Changed 12 years ago by
comment:2 Changed 12 years ago by
- Status changed from new to needs_review
The patch moves the two functions height_pairing_matrix and regulator_of_points, and adds doctests over number fields.
comment:3 Changed 12 years ago by
- Status changed from needs_review to needs_work
I'm getting a couple of doctest failures -- something is off by a sign in the height_pairing_matrix
code:
sage -t -long ell_number_field.py ********************************************************************** File "/storage/masiao/sage-4.4.4/devel/sage-reviewing/sage/schemes/elliptic_curves/ell_number_field.py", line 308: sage: E.height_pairing_matrix([P,Q]) Expected: [ 0.686667083305587 -0.268478098806726] [-0.268478098806726 0.327000773651605] Got: [0.686667083305587 0.268478098806726] [0.268478098806726 0.327000773651605] ********************************************************************** File "/storage/masiao/sage-4.4.4/devel/sage-reviewing/sage/schemes/elliptic_curves/ell_number_field.py", line 317: sage: EK.height_pairing_matrix([EK(P),EK(Q)]) Expected: [ 0.686667083305586 -0.268478098806726] [-0.268478098806726 0.327000773651605] Got: [0.686667083305586 0.268478098806726] [0.268478098806726 0.327000773651605] ********************************************************************** 1 items had failures: 2 of 23 in __main__.example_4 ***Test Failed*** 2 failures.
Also, a very tiny quibble: the second argument "precision" to height_pairing_matrix
is missing its bullet point in the docstring.
comment:4 Changed 12 years ago by
Probably I used E.gens() which is a bad idea in odctests, better to enter the points manually.
No time to fix now, about to leave SD22 for home.... but thanks all the same! Put this patch up from a coffee shop last night just before it closed (about a dozen Sagers being chased out!)
comment:5 Changed 12 years ago by
@cremona: You did you E.gens(). I get:
sage: E = EllipticCurve('389a1') sage: E.gens() [(-1 : 1 : 1), (0 : -1 : 1)]
Should I change the doc test to the following?
sage: E = EllipticCurve('389a1') sage: P,Q = E.point([-1,1,1]),E.point([0,-1,1]) sage: E.height_pairing_matrix([P,Q]) [0.686667083305587 0.268478098806726] [0.268478098806726 0.327000773651605]
}}}
comment:6 Changed 12 years ago by
- Status changed from needs_work to needs_review
If the only problem was that the doctest called E.gens(), this fixes those doctests and you can give this a positive review.
comment:7 Changed 12 years ago by
- Status changed from needs_review to positive_review
comment:8 Changed 12 years ago by
Thanks to all for sorting that out. I should have known better. Even for curves in the database, it is not safe to use gens() since unless you have the larger database installed the gens are computed on the fly and are not unique. (And doctests definitely should not assume an optional spkg is installed!).
comment:9 Changed 12 years ago by
- Merged in set to sage-4.5.2.alpha0
- Resolution set to fixed
- Reviewers set to David Loeffler, Robert Bradshaw
- Status changed from positive_review to closed
I'm updating the Reviewer(s) field. Please correct me if I'm wrong.
Applies to 4.4.4