Opened 7 years ago
Closed 7 years ago
#17495 closed defect (fixed)
Fix refine_embedding when some but not all embeddings are real
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage6.5 
Component:  number fields  Keywords:  
Cc:  Merged in:  
Authors:  Martin von Gagern  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  8aac6fa (Commits, GitHub, GitLab)  Commit:  8aac6fac1c438fca5defd9b4418c0ea0c541f7ea 
Dependencies:  Stopgaps: 
Description
The number field below has one real embedding and two complex embeddings. These complex embeddings should be ignored in refine_embedding
, but that's not the case, causing
sage: K.<a> = NumberField(x^3 + x  1, embedding=0.68) sage: from sage.rings.number_field.number_field import refine_embedding sage: refine_embedding(K.specified_complex_embedding(), 200) Traceback (most recent call last): ... TypeError: Unable to convert number to real interval.
Change History (11)
comment:1 Changed 7 years ago by
 Branch set to u/gagern/ticket/17495
 Created changed from 12/13/14 13:22:26 to 12/13/14 13:22:26
 Modified changed from 12/13/14 13:22:26 to 12/13/14 13:22:26
comment:2 Changed 7 years ago by
 Commit set to f30d693c593fcf8de8500c6821f2beff4c56c3a4
 Status changed from new to needs_review
comment:3 Changed 7 years ago by
 Commit changed from f30d693c593fcf8de8500c6821f2beff4c56c3a4 to cbe823a5dc0f0d31ada9afbf4003c386d964e221
Branch pushed to git repo; I updated commit sha1. New commits:
cbe823a  When determining embeddings, leave coefficients rational.

comment:4 Changed 7 years ago by
Regarding my second commit:
sage: f = QQ[x](69721504*x^8 + 251777664*x^6 + 329532012*x^4 + 184429548*x^2 + 37344321) sage: %timeit CDF[x](f).roots() 1000 loops, best of 3: 525 µs per loop sage: %timeit f.roots(CDF) 1000 loops, best of 3: 465 µs per loop sage: %timeit QQbar[x](f).roots() 10 loops, best of 3: 24.4 ms per loop sage: %timeit f.roots(QQbar) 10 loops, best of 3: 20.8 ms per loop
So using f.roots(K)
should be better than K['x'](f)
in pretty much all cases.
comment:5 Changed 7 years ago by
 Status changed from needs_review to needs_work
Please make this two lines:
r = f.roots(K, False); r.sort()
comment:6 Changed 7 years ago by
and use f.roots(K, multiplicities=False)
, which is much more explicit.
comment:7 Changed 7 years ago by
For the example you added, could you do it also for prec=Infinity
?
comment:8 Changed 7 years ago by
 Commit changed from cbe823a5dc0f0d31ada9afbf4003c386d964e221 to 8aac6fac1c438fca5defd9b4418c0ea0c541f7ea
Branch pushed to git repo; I updated commit sha1. New commits:
8aac6fa  Minor improvements regarding number field embeddings.

comment:9 Changed 7 years ago by
 Status changed from needs_work to needs_review
comment:10 Changed 7 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:11 Changed 7 years ago by
 Branch changed from u/gagern/ticket/17495 to 8aac6fac1c438fca5defd9b4418c0ea0c541f7ea
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
Only consider real embeddings if old embedding is into real lazy field.