Opened 7 years ago
Closed 7 years ago
#19912 closed defect (fixed)
Bug in method *to_cyclotomic_field* for the UniversalCyclotomicField
Reported by:  Christian Stump  Owned by:  

Priority:  major  Milestone:  sage7.1 
Component:  number fields  Keywords:  universal cyclotomic field 
Cc:  Vincent Delecroix, Frédéric Chapoton  Merged in:  
Authors:  Christian Stump  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  6f31818 (Commits, GitHub, GitLab)  Commit:  6f31818f26346cc05008c6ffeafe97b16f2cd156 
Dependencies:  Stopgaps: 
Description (last modified by )
sage: a = E(4); a E(4) sage: a.to_cyclotomic_field() zeta4 sage: a = 1+E(4); a 1 + E(4) sage: a.to_cyclotomic_field() zeta4
Change History (14)
comment:1 Changed 7 years ago by
Authors:  → Christian Stump 

Cc:  Vincent Delecroix Frédéric Chapoton added 
Component:  PLEASE CHANGE → number fields 
Description:  modified (diff) 
Keywords:  universal cyclotomic field added 
Type:  PLEASE CHANGE → defect 
comment:2 Changed 7 years ago by
Branch:  → u/stumpc5/19912 

comment:3 Changed 7 years ago by
Commit:  → 683a2f7fbd19f7d7508141732a6aa6b228276c47 

Status:  new → needs_review 
comment:4 Changed 7 years ago by
Reviewers:  → Vincent Delecroix 

Status:  needs_review → needs_work 
Hello Christian,
Indeed! The (1,k) should be changed in other places as well
sage: UCF = UniversalCyclotomicField() sage: UCFtoQQbar = UCF.coerce_embedding() sage: UCFtoQQbar(UCF.gen(4)+1) 1*I sage: UCFtoQQbar(UCF.gen(4)) 1*I
also
sage: CC(UCF.gen(4)) 1.00000000000000*I sage: CC(UCF.gen(4)+1) 1.00000000000000*I
Vincent
comment:5 Changed 7 years ago by
Commit:  683a2f7fbd19f7d7508141732a6aa6b228276c47 → 20d254c003a9353678bd52c265bf333802f7d616 

Branch pushed to git repo; I updated commit sha1. New commits:
20d254c  fixed the same bug in two more places

comment:6 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:7 Changed 7 years ago by
Status:  needs_review → needs_work 

Does not apply on the last develop.
comment:8 Changed 7 years ago by
Branch:  u/stumpc5/19912 → public/19912 

Commit:  20d254c003a9353678bd52c265bf333802f7d616 → d56fb17ff05647e9a3efb9775a1856c0b1eefdae 
I solved the conflict, and added the trac role.
New commits:
d56fb17  Merge branch 'u/stumpc5/19912' into 7.1.b0

comment:9 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:10 Changed 7 years ago by
Commit:  d56fb17ff05647e9a3efb9775a1856c0b1eefdae → 6f31818f26346cc05008c6ffeafe97b16f2cd156 

Branch pushed to git repo; I updated commit sha1. New commits:
6f31818  trac 19912 other trac roles

comment:11 Changed 7 years ago by
ok, looks good to me. Just waiting for the patchbot report to be sure.
comment:13 Changed 7 years ago by
Status:  needs_review → positive_review 

comment:14 Changed 7 years ago by
Branch:  public/19912 → 6f31818f26346cc05008c6ffeafe97b16f2cd156 

Resolution:  → fixed 
Status:  positive_review → closed 
Note: See
TracTickets for help on using
tickets.
New commits:
fixed small bug
added doctest for the bug