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: sage-7.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 mmezzarobba)

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 arminstraub

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:

sage: C.<z> = CyclotomicField(7)
sage: CC(z)
0.623489801858734 + 0.781831482468030*I
sage: QQbar(z)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<your snip>
TypeError: Illegal initializer for algebraic number

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.)

comment:2 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:3 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:5 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:6 Changed 3 years ago by mmezzarobba

  • Authors set to Marc Mezzarobba
  • 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:

b0cbce7conversion of number field elts with real/complex embeddings to algebraic numbers
526e66cadd test from #13041

comment:7 Changed 3 years ago by mmezzarobba

  • 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 follow-up: Changed 3 years ago by vdelecroix

  • Milestone changed from sage-6.4 to sage-7.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 mmezzarobba

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 git

  • Commit changed from 526e66c778c58e34086efd6ad5871ea84186520d to ee1fc05236e7a68d09bcd0070a58e3b038f69c5e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ee1fc05conversion of number field elts with real/complex embeddings to algebraic numbers

comment:11 Changed 3 years ago by git

  • Commit changed from ee1fc05236e7a68d09bcd0070a58e3b038f69c5e to b28381af685fb5238ce1e68af834bf61ba23e936

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b28381aconversion of number field elts with real/complex embeddings to algebraic numbers

comment:12 Changed 3 years ago by mmezzarobba

  • 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:

b28381aconversion of number field elts with real/complex embeddings to algebraic numbers

comment:13 Changed 3 years ago by vdelecroix

  • 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 mmezzarobba

Thanks for the review!

comment:15 Changed 3 years ago by vbraun

  • Branch changed from u/mmezzarobba/NFtoQQbar to b28381af685fb5238ce1e68af834bf61ba23e936
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.