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

abs for elements of number fields provided with a complex embedding ignores it #16147

Closed
sagetrac-fwclarke mannequin opened this issue Apr 12, 2014 · 8 comments
Closed

Comments

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Apr 12, 2014

See http://ask.sagemath.org/question/3967/qqextension-with-embedding-incorrect-modulus

sage: x = polygen(ZZ)
sage: f = x^3 - x - 1
sage: beta = f.complex_roots()[0]; beta
1.32471795724475
sage: K.<b> = NumberField(f, embedding=beta)
sage: b.abs()
0.868836961832709
sage: [r.abs() for r in f.complex_roots()]
[1.32471795724475, 0.868836961832709, 0.868836961832709]

Component: number fields

Author: Vincent Delecroix

Branch/Commit: 56042dc

Reviewer: Francis Clarke

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

@sagetrac-fwclarke sagetrac-fwclarke mannequin added this to the sage-6.2 milestone Apr 12, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@videlec
Copy link
Contributor

videlec commented Jun 29, 2014

comment:2

I provided a simple fix... needs review!

Vincent


New commits:

56042dctrac #16147: fix .abs() for elt of embedded number field

@videlec
Copy link
Contributor

videlec commented Jun 29, 2014

Branch: u/vdelecroix/16147

@videlec
Copy link
Contributor

videlec commented Jun 29, 2014

Author: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Jun 29, 2014

Commit: 56042dc

@sagetrac-fwclarke
Copy link
Mannequin Author

sagetrac-fwclarke mannequin commented Jun 29, 2014

comment:3

Does the job very neatly: positive review.

@videlec
Copy link
Contributor

videlec commented Jun 29, 2014

Reviewer: Francis Clarke

@videlec
Copy link
Contributor

videlec commented Jun 29, 2014

comment:4

Replying to @sagetrac-fwclarke:

Does the job very neatly: positive review.

Great! thanks!

When you finish the review, you might better fill the reviewer field with your name (otherwise the release manager has extra job to do). It should be filled with your full name. I did it for you anyway.

Vincent

PS: Actually, I thought that the default answer of .abs() could be an element of QQbar (whenever an embedding in QQbar is defined). That way we would have an exact algebraic number and not an approximation. But in order to do that, there is something to fix with embeddings:

sage: K.<cbrt2> = NumberField(x^3 - 2, embedding=1)
sage: QQbar.has_coerce_map_from(K)
False

@vbraun
Copy link
Member

vbraun commented Jun 30, 2014

Changed branch from u/vdelecroix/16147 to 56042dc

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

2 participants