sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.33k stars 453 forks source link

elliptic curves -- some period lattice functions are not implemented #1904

Closed williamstein closed 13 years ago

williamstein commented 16 years ago
sage: E = EllipticCurve('37a1')
sage: Lambda = E.period_lattice()
sage: OE = Lambda.omega(); OE
5.986917292463919259664019958905016355595167582740265970681046757126500713973
sage: Lambda.matrix()
Traceback (most recent call last):
...
TypeError: Unable to coerce 2.451389381986790060854224831866525225349617289144796614656471406129152899999*I (<type 'sage.rings.complex_number.ComplexNumber'>) to Rational
sage: Lambda.gram_matrix()
Traceback (most recent call last):
...
AttributeError: 'PeriodLattice_ell' object has no attribute 'ambient_vector_space'
sage: Lambda.basis()
(2.993458646231959629832009979452508177797583791370132985340523378563250356987, 2.451389381986790060854224831866525225349617289144796614656471406129152899999*I)
sage: Lambda.basis_matrix()
Traceback (most recent call last):
...
TypeError: Unable to coerce 2.451389381986790060854224831866525225349617289144796614656471406129152899999*I (<type 'sage.rings.complex_number.ComplexNumber'>) to Rational
s

The root cause of this is that Period lattices actually derive from the abstract free module type, but they don't implement all the functionality that type requires.

CC: @loefflerd

Component: elliptic curves

Keywords: ecc2011

Reviewer: John Cremona, David Loeffler, Paul Zimmermann

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

JohnCremona commented 16 years ago
comment:1

Just to confirm that none of this has changed yet despite a lot of (mainly precision-related) work on period lattices leading up to 3.1.2.

I'm not sure what benefits, if any, the class PeriodLattice_ell gains from being derived from FreeModule_generic_pid (via class PeriodLattice), but it means that len(dir(L)) is 210 so there's a lot of work to do implementing possibly trivial and possibly irrelevant things.

The issue is that a FreeModule_generic_pid seems to think that it's a submodule of some concrete module like ZZ^n, rather than being an abstract module; functions like basis_matrix() make no sense in general.

If anyone can point out a method of FreeModule_generic_pid which PeriodLattice_ell should implement but does not, I would be happy to implement it.

11d1fc49-71a1-44e1-869f-76be013245a0 commented 15 years ago
comment:2

Just for the record: ticket #4388 goes some way towards fixing this by providing a basis_matrix() method for period lattices. This fixes all of the problems reported by was above, but several issues remain: for example, I can't seem to create any nonzero element of a period lattice -- the return values of basis() are plain complex numbers, not elements of the lattice, and attempting to coerce them back into the lattice fails.

JohnCremona commented 15 years ago
comment:3

You are right, David. For me this class is just a sort of place-holder to hold the data needed for when you ask an elliptic curve about its periods and related things. I never thought about it as a lattice in any precise sense. (I did not create the class, by the way, but I have added to it -- for example, support for real embeddings of number fields, not just Q).

Feel free to let it behave more sensibly for what you need by adding stuff! John

11d1fc49-71a1-44e1-869f-76be013245a0 commented 15 years ago
comment:5

Assigned to new "elliptic curves" component.

zimmermann6 commented 13 years ago

Changed keywords from none to ecc2011

zimmermann6 commented 13 years ago
comment:7

the examples in the description work with Sage 4.7.1:

----------------------------------------------------------------------
| Sage Version 4.7.1, Release Date: 2011-08-11                       |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
Loading Sage library. Current Mercurial branch is: 9322
sage: E = EllipticCurve('37a1')
sage: Lambda = E.period_lattice()
sage: OE = Lambda.omega(); OE
5.98691729246392
sage: Lambda.matrix()
[ 2.99345864623196 0.000000000000000]
[0.000000000000000  2.45138938198679]
sage: Lambda.gram_matrix()
[ 8.96079466670088 0.000000000000000]
[0.000000000000000  6.00930990211758]
sage: Lambda.basis()
(2.99345864623196, 2.45138938198679*I)
sage: Lambda.basis_matrix()
[ 2.99345864623196 0.000000000000000]
[0.000000000000000  2.45138938198679]

Should this ticket be closed?

Paul

JohnCremona commented 13 years ago
comment:8

Replying to @zimmermann6:

Should this ticket be closed?

Paul

In my opinion, yes, but see the comments above by David Loeffler.

11d1fc49-71a1-44e1-869f-76be013245a0 commented 13 years ago
comment:9

I concur with the vote to close. Setting this to "positive review" to bring it to the notice of someone who has the authority to do so. -- David

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 13 years ago

Reviewer: John Cremona, David Loeffler, Paul Zimmermann