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

Priority:  major  Milestone:  sage7.1 
Component:  number fields  Keywords:  universal cyclotomic field 
Cc:  vdelecroix, 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 5 years ago by
 Cc vdelecroix chapoton added
 Component changed from PLEASE CHANGE to number fields
 Description modified (diff)
 Keywords universal cyclotomic field added
 Type changed from PLEASE CHANGE to defect
comment:2 Changed 5 years ago by
 Branch set to u/stumpc5/19912
comment:3 Changed 5 years ago by
 Commit set to 683a2f7fbd19f7d7508141732a6aa6b228276c47
 Status changed from new to needs_review
comment:4 Changed 5 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to 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 5 years ago by
 Commit changed from 683a2f7fbd19f7d7508141732a6aa6b228276c47 to 20d254c003a9353678bd52c265bf333802f7d616
Branch pushed to git repo; I updated commit sha1. New commits:
20d254c  fixed the same bug in two more places

comment:6 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:7 Changed 5 years ago by
 Status changed from needs_review to needs_work
Does not apply on the last develop.
comment:8 Changed 5 years ago by
 Branch changed from u/stumpc5/19912 to public/19912
 Commit changed from 20d254c003a9353678bd52c265bf333802f7d616 to 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 5 years ago by
 Status changed from needs_work to needs_review
comment:10 Changed 5 years ago by
 Commit changed from d56fb17ff05647e9a3efb9775a1856c0b1eefdae to 6f31818f26346cc05008c6ffeafe97b16f2cd156
Branch pushed to git repo; I updated commit sha1. New commits:
6f31818  trac 19912 other trac roles

comment:11 Changed 5 years ago by
ok, looks good to me. Just waiting for the patchbot report to be sure.
comment:12 Changed 5 years ago by
Thanks  my machine is still compiling 7.1.beta0
...
comment:13 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:14 Changed 5 years ago by
 Branch changed from public/19912 to 6f31818f26346cc05008c6ffeafe97b16f2cd156
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
fixed small bug
added doctest for the bug