Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in method *to_cyclotomic_field* for the UniversalCyclotomicField #19912

Closed
stumpc5 opened this issue Jan 19, 2016 · 20 comments
Closed

Bug in method *to_cyclotomic_field* for the UniversalCyclotomicField #19912

stumpc5 opened this issue Jan 19, 2016 · 20 comments

Comments

@stumpc5
Copy link
Contributor

stumpc5 commented Jan 19, 2016

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

CC: @videlec @fchapoton

Component: number fields

Keywords: universal cyclotomic field

Author: Christian Stump

Branch/Commit: 6f31818

Reviewer: Vincent Delecroix

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

@stumpc5 stumpc5 added this to the sage-7.1 milestone Jan 19, 2016
@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 19, 2016

Author: Christian Stump

@stumpc5

This comment has been minimized.

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 19, 2016

Changed keywords from none to universal cyclotomic field

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 19, 2016

Branch: u/stumpc5/19912

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 19, 2016

New commits:

3b24329fixed small bug
683a2f7added doctest for the bug

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 19, 2016

Commit: 683a2f7

@videlec
Copy link
Contributor

videlec commented Jan 19, 2016

Reviewer: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Jan 19, 2016

comment:4

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

20d254cfixed the same bug in two more places

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2016

Changed commit from 683a2f7 to 20d254c

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 19, 2016

New commits:

20d254cfixed the same bug in two more places

New commits:

20d254cfixed the same bug in two more places

@videlec
Copy link
Contributor

videlec commented Jan 19, 2016

comment:7

Does not apply on the last develop.

@fchapoton
Copy link
Contributor

comment:8

I solved the conflict, and added the trac role.


New commits:

d56fb17Merge branch 'u/stumpc5/19912' into 7.1.b0

@fchapoton
Copy link
Contributor

Changed branch from u/stumpc5/19912 to public/19912

@fchapoton
Copy link
Contributor

Changed commit from 20d254c to d56fb17

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

6f31818trac 19912 other trac roles

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2016

Changed commit from d56fb17 to 6f31818

@fchapoton
Copy link
Contributor

comment:11

ok, looks good to me. Just waiting for the patchbot report to be sure.

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 27, 2016

comment:12

Thanks -- my machine is still compiling 7.1.beta0...

@vbraun
Copy link
Member

vbraun commented Jan 28, 2016

Changed branch from public/19912 to 6f31818

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants