Opened 4 years ago
Closed 4 years ago
#23599 closed enhancement (fixed)
making local_data and check_prime from Elliptic curves to accept P in QQ
Reported by:  edgarcosta  Owned by:  

Priority:  minor  Milestone:  sage8.1 
Component:  elliptic curves  Keywords:  days88 
Cc:  cremona  Merged in:  
Authors:  Edgar Costa  Reviewers:  Aly Deines 
Report Upstream:  N/A  Work issues:  
Branch:  58b7fbd (Commits, GitHub, GitLab)  Commit:  58b7fbd209d8d7f3850df4ab6f20eeaf3e7f5e2a 
Dependencies:  Stopgaps: 
Description
I noticed that one could do.
E = EllipticCurve(QQ, "35a2") E.local_data(ZZ(5));
but not
E.local_data(QQ(5));
However, if we base change to a number field K
, it works if we pass an element of K
K.<i> = NumberField(x^2+1) E = EllipticCurve([1 + i, 0, 1, 0, 0]) E.local_data(2*i + 1) E.local_data(K(3))
but
E.local_data(3)
woudln't.
I tried to fix that.
ps: the right implementation here should be along the lines,
P = K.fractional_ideal(P) if P.is_prime(): if K is QQ: return P.gen(); else: return P;
but QQ
doesn't have fractional ideals, see #23595
Change History (10)
comment:1 Changed 4 years ago by
 Status changed from new to needs_review
comment:2 Changed 4 years ago by
comment:3 Changed 4 years ago by
Ah, now I see. Of course that is what you want, P=QQ(3) gives now True. Sorry for the noise.
comment:4 Changed 4 years ago by
I approve of these changes (in principle, I have not looked at the details). There are still many places where QQ (and elements of QQ) are not treated the same as for number fields, and missing methods for QQ and rationals which exist for general number fields. I don't think it will ever be possible to eliminate these inconsistencies globally for a number of reasons:
 We have different types (classes) for elements of QQ and ZZ, and that will surely stay true, while we have no separate type for an integral element of a number field.
 Number theorists are quite happy with K.ideal(2) returning a fractional ideal  which is nontrivial!  but QQ.ideal(2) returns the unit ideal. Basically that's because algebraists are pedantic and say quite correctly that the ideal of QQ generated by 2 is the unit ideal since 2 is a unit in the field. Of course we can do ZZ.ideal(2) to get the ideal; but not ZZ.ideal(1/2), i.e. we have integral ideals of ZZ but not fractional ones. So this is in fact related to the distinct classes for ZZ and QQ but none for rings of integers generally.
Hence (in conclusion!) the best we can do is tinker with code in the way this ticket does so that natural constructions work as widely as possible. Thanks, Edgar.
comment:5 Changed 4 years ago by
 Keywords days88 added
comment:6 Changed 4 years ago by
 Branch changed from u/edgarcosta/local_data to u/aly.deines/local_data
comment:7 Changed 4 years ago by
 Commit changed from 5976388e6527d821158d37796c5251608f64de00 to 58b7fbd209d8d7f3850df4ab6f20eeaf3e7f5e2a
 Reviewers set to Aly Deines
 Status changed from needs_review to positive_review
Merged.
Plays well. All tests pass. Documentation looks good.
New commits:
58b7fbd  Merge branch 'u/edgarcosta/local_data' of git://trac.sagemath.org/sage into t/23599/local_data

comment:8 Changed 4 years ago by
 Status changed from positive_review to needs_work
Author name missing...
comment:9 Changed 4 years ago by
 Status changed from needs_work to positive_review
comment:10 Changed 4 years ago by
 Branch changed from u/aly.deines/local_data to 58b7fbd209d8d7f3850df4ab6f20eeaf3e7f5e2a
 Resolution set to fixed
 Status changed from positive_review to closed
Not that I can claim to understand types in sage completely, but your change:
seems to complicate a line without changing the criterion, no?