Opened 7 years ago
Closed 3 years ago
#13041 closed defect (fixed)
Conversion of number field elements to algebraic numbers
Reported by:  rbeezer  Owned by:  AlexGhitza 

Priority:  minor  Milestone:  sage7.2 
Component:  algebra  Keywords:  sd40.5 
Cc:  Merged in:  
Authors:  Marc Mezzarobba  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  b28381a (Commits)  Commit:  b28381af685fb5238ce1e68af834bf61ba23e936 
Dependencies:  Stopgaps: 
Description (last modified by )
Implement a basic (but better than nothing) conversion of elements of number fields embedded into the complex numbers to elements of QQbar
.
Origininal report:
sage: C.<z> = CyclotomicField(7) sage: a = 2*z^2 + 5*z^4 sage: E = C.algebraic_closure() sage: E(a)  TypeError Traceback (most recent call last) <snip> TypeError: Illegal initializer for algebraic number
Change History (15)
comment:1 Changed 7 years ago by
comment:2 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:3 Changed 6 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:4 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:5 Changed 5 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:6 Changed 3 years ago by
 Branch set to u/mmezzarobba/NFtoQQbar
 Commit set to 526e66c778c58e34086efd6ad5871ea84186520d
 Status changed from new to needs_review
I agree that this should be solved by #5355/#12715, but #5355 has been open for seven years now... The attached branch provides a temporary fix that at least makes the most common use case work, and shouldn't be hard to remove once a proper solution is in place.
New commits:
b0cbce7  conversion of number field elts with real/complex embeddings to algebraic numbers

526e66c  add test from #13041

comment:7 Changed 3 years ago by
 Description modified (diff)
 Summary changed from Cannot convert elements of cyclotomic fields to algebraic numbers to Conversion of number field elements to algebraic numbers
comment:8 followup: ↓ 9 Changed 3 years ago by
 Milestone changed from sage6.4 to sage7.2
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to needs_work
Why you can not go directly NumberField > IntervalField
? (i.e. the FIXME in comment). Is there a related ticket? Indeed, the code will not work in some cases because of the following
sage: K = NumberField(x^3  2, 'cbrt2', embedding=RIF(1.25,1.26)) sage: RLF(K.gen()) Traceback (most recent call last): ... RuntimeError: maximum recursion depth exceeded while calling a Python object
comment:9 in reply to: ↑ 8 Changed 3 years ago by
Replying to vdelecroix:
Why you can not go directly
NumberField > IntervalField
? (i.e. the FIXME in comment).
For example:
sage: C.<z> = CyclotomicField(7) sage: a = 2*z^2 + 5*z^4 sage: CIF(a) ... TypeError: unable to coerce to a ComplexIntervalFieldElement sage: CIF(CLF(a)) 4.9498862074248?  0.2195628712242?*I
Is there a related ticket?
I don't know. I needed to convert number field elements to algebraic numbers in experimental code of mine, so I wrote a small patch to make that work in the cases I was interested in, but I didn't try to go to the source of the problem...
Indeed, the code will not work in some cases because of the following
sage: K = NumberField(x^3  2, 'cbrt2', embedding=RIF(1.25,1.26)) sage: RLF(K.gen()) Traceback (most recent call last): ... RuntimeError: maximum recursion depth exceeded while calling a Python object
Yes, the patch is far from perfect. Even so, I think it is an improvement (and I don't have much time to work on a cleaner fix right now), but if you disagree, I'll just keep my local patch for now!
comment:10 Changed 3 years ago by
 Commit changed from 526e66c778c58e34086efd6ad5871ea84186520d to ee1fc05236e7a68d09bcd0070a58e3b038f69c5e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ee1fc05  conversion of number field elts with real/complex embeddings to algebraic numbers

comment:11 Changed 3 years ago by
 Commit changed from ee1fc05236e7a68d09bcd0070a58e3b038f69c5e to b28381af685fb5238ce1e68af834bf61ba23e936
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b28381a  conversion of number field elts with real/complex embeddings to algebraic numbers

comment:12 Changed 3 years ago by
 Status changed from needs_work to needs_review
I found a bit of time after all, and I realized that number_field.refine_embedding
already does most of the job. So here is another attempt. (I only ran the tests in rings
.)
New commits:
b28381a  conversion of number field elts with real/complex embeddings to algebraic numbers

comment:13 Changed 3 years ago by
 Status changed from needs_review to positive_review
All right. As the situation gets better with the branch, let's include it.
comment:14 Changed 3 years ago by
Thanks for the review!
comment:15 Changed 3 years ago by
 Branch changed from u/mmezzarobba/NFtoQQbar to b28381af685fb5238ce1e68af834bf61ba23e936
 Resolution set to fixed
 Status changed from positive_review to closed
Unfortunately, at the moment, elements of number fields don't coerce/convert to the algebraic closure QQbar. This is an instance of #12715 (Number field embeddings should go via AA and QQbar).
Note that you can coerce elements to numerical complex numbers though:
I would therefore suggest to mark this ticket as a duplicate. (I don't immediately see a way to do that myself, so please let me know if I could and should have done so myself.)