#20181 closed enhancement (fixed)
number_field_elements_from_algebraics should create embedded number field elements
Reported by:  mkoeppe  Owned by:  

Priority:  minor  Milestone:  sage8.8 
Component:  number fields  Keywords:  IMAPolyGeom 
Cc:  vbraun, gagern, vdelecroix, tmonteil, jj, sstarosta, jipilab, moritz, yzh, cremona, tscrim  Merged in:  
Authors:  JeanPhilippe Labbé  Reviewers:  Matthias Koeppe, John Cremona 
Report Upstream:  N/A  Work issues:  
Branch:  9255f2c (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
This is a followup on #17830 and #18819.
#17830 made this possible:
sage: x = polygen(ZZ) sage: K.<cbrt2> = NumberField(x^3  2, embedding=AA.polynomial_root(x^32, RIF(0,3))) sage: 6064/4813 < cbrt2 < 90325/71691 True
But if one uses number_field_elements_from_algebraics
, one gets only nonembedded number field elements:
sage: x = polygen(ZZ) sage: K, cbrt2, hom = number_field_elements_from_algebraics(AA.polynomial_root(x^32, RIF(0,3))) sage: 6064/4813 < cbrt2 < 90325/71691 False # (an perhaps an error in Python 3?)
This ticket adds an option embedded
to create embedded number field elements.
sage: x = polygen(ZZ) sage: K, cbrt2, hom = number_field_elements_from_algebraics(AA.polynomial_root(x^32, RIF(0,3)), embedded=True) sage: 6064/4813 < cbrt2 < 90325/71691 True
TODECIDE:
 Close ticket #5355?
Change History (55)
comment:1 Changed 5 years ago by
comment:2 Changed 5 years ago by
 Description modified (diff)
comment:3 Changed 2 years ago by
 Cc jipilab moritz added
comment:4 Changed 2 years ago by
 Cc yzh added
comment:5 Changed 2 years ago by
 Branch set to public/algnum_to_numberfield
 Commit set to 26dc3e4e6a26f6f613a69d57929ea492c278dad0
 Status changed from new to needs_info
Here is a first version. Comments are welcome.
One question is whether RealIntervalField
is the right choice?
New commits:
26dc3e4  Added option for embedded NumberField

comment:6 Changed 2 years ago by
Maybe it would be nice to also add the optional parameter to the method as_number_field_element
as well...
comment:7 Changed 2 years ago by
The following should work but it does not:
sage: UCF = UniversalCyclotomicField() sage: E = UCF.gen(5) sage: my_nums = [52*E  136*E^2  136*E^3  52*E^4, 66*E  168*E^2  168*E^3  66*E^4, 46*E  114*E^2  114*E^3  46*E^4, 24*E  58*E^2  58*E ....: ^3  24*E^4, sqrt(2)] sage: [n(_) for _ in my_nums] [187.914855054991  8.67361737988404e18*I, 231.039466852489  1.04083408558608e17*I, 156.026311234993  8.67361737988404e18*I, 79.0131556174964  4.33680868994202e18*I, 1.41421356237310] sage: res = number_field_elements_from_algebraics(my_nums,embedded=True) Traceback (most recent call last): ... TypeError: unable to convert 0.4370160244488211?  1.344997023927915?*I to real interval
The choice of root shoould always be made real...
comment:8 Changed 2 years ago by
 Commit changed from 26dc3e4e6a26f6f613a69d57929ea492c278dad0 to db778eeeb93978335aa113e09a51ccabfe2350e4
Branch pushed to git repo; I updated commit sha1. New commits:
db778ee  Made it possible to deal with complex number fields

comment:9 Changed 2 years ago by
It is now possible to get this:
sage: UCF = UniversalCyclotomicField() sage: E = UCF.gen(5) sage: L.<b> = NumberField(x^2189*x+16, embedding=200) sage: my_nums = [52*E  136*E^2  136*E^3  52*E^4, L.gen()._algebraic_(AA),sqrt(2)] sage: aa_my_nums = [AA(_) for _ in my_nums] sage: res = number_field_elements_from_algebraics(aa_my_nums,embedded=True) sage: res (Number Field in a with defining polynomial y^8  35670*y^6 + 476899047*y^4  2832410271650*y^2 + 6305298701739921, [2310/26212773509*a^7  185432947/78638320527*a^5 + 1652517502195/78638320527*a^3  4904676315215467/78638320527*a + 94, 1238/2803377488467023*a^7 + 185460719/11213509953868092*a^5  2754936849443/11213509953868092*a^3 + 8180694680816975/3737836651289364*a + 189/2, 1979/1887160880826*a^7 + 26472586/943580440413*a^5  235822245043/943580440413*a^3 + 466325019915415/629053626942*a], Ring morphism: From: Number Field in a with defining polynomial y^8  35670*y^6 + 476899047*y^4  2832410271650*y^2 + 6305298701739921 To: Algebraic Real Field Defn: a > 96.9475535136628?) sage: res[0].gen_embedding() 96.9475535136628? sage: res = number_field_elements_from_algebraics(aa_my_nums,embedded=False) sage: res[0].gen_embedding() # Nothing is returned(!?!?!) sage:
Perhaps it would be nice to say that there is no embedding instead of returning nothing?
comment:10 Changed 2 years ago by
 Status changed from needs_info to needs_review
comment:11 Changed 2 years ago by
comment:12 followup: ↓ 17 Changed 2 years ago by
The returned morphism is not able to convert the elements.
sage: res[2](my_nums[0])  TypeError Traceback (most recent call last) <ipythoninput874370e91d982> in <module>() > 1 res[Integer(2)](my_nums[Integer(0)]) /Users/mkoeppe/s/sage/somesageinstallprefix/lib/python2.7/sitepackages/sage/categories/map.pyx in sage.categories.map.Map.__call__ (build/cythonized/sage/categories/map.c:6978)() 781 x = D(x) 782 except (TypeError, NotImplementedError): > 783 raise TypeError("%s fails to convert into the map's domain %s, but a `pushforward` method is not properly implemented" % (x, D)) 784 else: 785 x = converter(x) TypeError: 52*E(5)  136*E(5)^2  136*E(5)^3  52*E(5)^4 fails to convert into the map's domain Number Field in a with defining polynomial y^8  35670*y^6 + 476899047*y^4  2832410271650*y^2 + 6305298701739921, but a `pushforward` method is not properly implemented
Edit: I withdraw this comment; I misremembered what hom
is supposed to do.
comment:13 followup: ↓ 14 Changed 2 years ago by
Two quick comments:
 you may want to pathc
as_number_field_element()
too,  don't forget to add tests covering the case where some or all of the elements are rationals or elements of quadratic extensions (rationals, quadratic number field elements and higherdegree number field elements don't have exactly the same interface...)
comment:14 in reply to: ↑ 13 Changed 2 years ago by
Replying to mmezzarobba:
Two quick comments:
 you may want to pathc
as_number_field_element()
too, don't forget to add tests covering the case where some or all of the elements are rationals or elements of quadratic extensions (rationals, quadratic number field elements and higherdegree number field elements don't have exactly the same interface...)
Yes, that sounds very reasonable. I will do that, along with fixing what mkoeppe mentionned above.
comment:15 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:16 Changed 2 years ago by
Doesn't always seem to give real embeddings:
sage: x = polygen(ZZ); elts = number_field_elements_from_algebraics([sqrt(2), AA.polynomial_root(x^32, RIF(0,3))], embedded=True)[1] sage: [AA(y) for y in elts] ... TypeError: unable to convert 0.561231024154687?  0.9720806486198328?*I to real interval
comment:17 in reply to: ↑ 12 Changed 2 years ago by
Replying to mkoeppe:
The returned morphism is not able to convert the elements.
sage: res[2](my_nums[0])  TypeError Traceback (most recent call last) <ipythoninput874370e91d982> in <module>() > 1 res[Integer(2)](my_nums[Integer(0)]) /Users/mkoeppe/s/sage/somesageinstallprefix/lib/python2.7/sitepackages/sage/categories/map.pyx in sage.categories.map.Map.__call__ (build/cythonized/sage/categories/map.c:6978)() 781 x = D(x) 782 except (TypeError, NotImplementedError): > 783 raise TypeError("%s fails to convert into the map's domain %s, but a `pushforward` method is not properly implemented" % (x, D)) 784 else: 785 x = converter(x) TypeError: 52*E(5)  136*E(5)^2  136*E(5)^3  52*E(5)^4 fails to convert into the map's domain Number Field in a with defining polynomial y^8  35670*y^6 + 476899047*y^4  2832410271650*y^2 + 6305298701739921, but a `pushforward` method is not properly implemented
The homomorphism goes the other way around... Thanks to Moritz for noticing that.
comment:18 Changed 2 years ago by
 Commit changed from db778eeeb93978335aa113e09a51ccabfe2350e4 to a9045bf8a3aab2d0aa00be17e91227bc1b50262a
Branch pushed to git repo; I updated commit sha1. New commits:
a9045bf  Added tests and made them pass

comment:19 Changed 2 years ago by
Okay. I added some tests and now it seems to work.
With the current commit, I have one failing doctest for which I do not know what to do:
File "qqbar.py", line 2098, in sage.rings.qqbar.number_field_elements_from_algebraics Failed example: number_field_elements_from_algebraics(rt2c) Expected: (Number Field in a with defining polynomial y^4 + 2*y^2 + 4, 1/2*a^3, Ring morphism: From: Number Field in a with defining polynomial y^4 + 2*y^2 + 4 To: Algebraic Field Defn: a > 0.7071067811865475?  1.224744871391589?*I) Got: (Number Field in a with defining polynomial y^2  2, a, Ring morphism: From: Number Field in a with defining polynomial y^2  2 To: Algebraic Real Field Defn: a > 1.414213562373095?)
This is in the tests section and the explanation given is exactly a situation that we have to outsmart when we want to be able to accept cyclotomic fields elements to be parsed to real embedded number fields.
It seems to me that the proposed code improves the situation as Sage now recognize that it is a real number... I can not see a reason right now to go against that. Am I missing something?
comment:20 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:21 Changed 2 years ago by
 Keywords IMAPolyGeom added
comment:22 Changed 2 years ago by
 Milestone changed from sage7.1 to sage8.3
comment:23 followup: ↓ 33 Changed 2 years ago by
 Cc cremona added
Modifying the test in comment 19 looks OK to me, but perhaps John can confirm I'm not missing anything...
comment:24 Changed 2 years ago by
You should just go on and fix the doctest, I think.
comment:25 Changed 2 years ago by
 Reviewers set to Matthias Koeppe
 Status changed from needs_review to needs_work
as_number_field_element
needs to pass on its new arguments, and corresponding doctests should be added.
comment:26 Changed 2 years ago by
 Commit changed from a9045bf8a3aab2d0aa00be17e91227bc1b50262a to 60c527f8ef3f2ed7bb75220fceeb532ff7096120
Branch pushed to git repo; I updated commit sha1. New commits:
60c527f  Merge remotetracking branch 'trac/develop' into t/20181/public/algnum_to_numberfield

comment:27 Changed 2 years ago by
 Commit changed from 60c527f8ef3f2ed7bb75220fceeb532ff7096120 to f1627e7a51c74327cfb91e37eaecadd6b3aee173
Branch pushed to git repo; I updated commit sha1. New commits:
f1627e7  AlgebraicNumber_base.as_number_field_element: Pass new kwargs to number_field_elements_from_algebraics

comment:28 Changed 2 years ago by
... still needs a corresponding doctest
comment:29 Changed 2 years ago by
 Commit changed from f1627e7a51c74327cfb91e37eaecadd6b3aee173 to 41b1142066d3c718006ee3363d41f4367949bc03
Branch pushed to git repo; I updated commit sha1. New commits:
d7e31a2  AlgebraicNumber_base.as_number_field_element: Add doctest

284c217  Merge remotetracking branch 'trac/develop' into t/20181/public/algnum_to_numberfield

41b1142  AlgebraicNumber_base.as_number_field_element: Add doctests (currently failing)

comment:30 Changed 2 years ago by
I've added some doctests, but the implementation needs more work as there are failures.
comment:31 Changed 2 years ago by
 Cc tscrim added
 Description modified (diff)
 Milestone changed from sage8.3 to sage8.4
comment:32 Changed 2 years ago by
 Commit changed from 41b1142066d3c718006ee3363d41f4367949bc03 to 2eb332053089cccf9266850d2fbd964a3c24b9fe
Branch pushed to git repo; I updated commit sha1. New commits:
2eb3320  number_field_elements_from_algebraics: Mark a random doctest # random

comment:33 in reply to: ↑ 23 Changed 2 years ago by
Replying to dimpase:
Modifying the test in comment 19 looks OK to me, but perhaps John can confirm I'm not missing anything...
The context of this doctest suggested that it should be marked # random
, which I did in 2eb3320.
comment:34 Changed 2 years ago by
Ready for rereview?
Some quick comments. It is considered a bad practice to have bare except:
statements. It is probably sufficient to catch ValueError
and TypeError
. Also, the standard doc format for INPUT:
would be
 ``numbers``  a number or list of numbers  ``minimal``  boolean (default: ``False``); whether to minimize the degree of the extension  ``same_field``  boolean (default: ``False``); see below  ``embedded``  boolean (default: ``False``); whether to make the :func:`NumberField` embedded  ``prec``  integer (default: ``53``); the number of bit of precision for the embedding
 ``minimal``  boolean (default: ``False``); whether to minimize the degree of the extension  ``embedded``  boolean (default: ``False``); whether to make the :func:`NumberField` embedded  ``prec``  integer (default: ``53``); the number of bit of precision for the embedding
comment:35 followup: ↓ 36 Changed 2 years ago by
Not ready for review; this needs more work  see the failing doctests that I added in 41b1142.
comment:36 in reply to: ↑ 35 Changed 2 years ago by
Replying to mkoeppe:
Not ready for review; this needs more work  see the failing doctests that I added in 41b1142.
For reference: These are the doctest failures.
$ sage t src/sage/rings/qqbar.py too many failed tests, not using stored timings Running doctests with ID 2018092716591065572ec7. Using optional=4ti2,bliss,ccache,dochtml,gdb,latte_int,lidia,lrslib,mpir,normaliz,notedown,pandoc_attributes,pynormaliz,pyqnormaliz,python2,rst2ipynb,sage Doctesting 1 file. sage t src/sage/rings/qqbar.py ********************************************************************** File "src/sage/rings/qqbar.py", line 3696, in sage.rings.qqbar.AlgebraicNumber_base.as_number_field_element Failed example: elt == rt Expected: True Got: False ********************************************************************** File "src/sage/rings/qqbar.py", line 3698, in sage.rings.qqbar.AlgebraicNumber_base.as_number_field_element Failed example: AA(elt) Exception raised: Traceback (most recent call last): File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 659, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 1070, in compile_and_execute exec(compiled, globs) File "<doctest sage.rings.qqbar.AlgebraicNumber_base.as_number_field_element[12]>", line 1, in <module> AA(elt) File "sage/structure/parent.pyx", line 921, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9679) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 145, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4574) raise File "sage/structure/coerce_maps.pyx", line 140, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4442) return C._element_constructor(x) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/rings/qqbar.py", line 738, in _element_constructor_ return x._algebraic_(AA) File "sage/rings/number_field/number_field_element.pyx", line 2709, in sage.rings.number_field.number_field_element.NumberFieldElement._algebraic_ (build/cythonized/sage/rings/number_field/number_field_element.cpp:26930) emb = refine_embedding(emb, infinity) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/rings/number_field/number_field.py", line 11329, in refine_embedding diffs = [(RC(ee(K.gen()))old_root).abs() for ee in elist] File "sage/rings/real_mpfi.pyx", line 699, in sage.rings.real_mpfi.RealIntervalField_class.__call__ (build/cythonized/sage/rings/real_mpfi.c:6094) return Parent.__call__(self, x) File "sage/structure/parent.pyx", line 921, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9679) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 271, in sage.structure.coerce_maps.NamedConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:5999) cdef Element e = method(C) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/rings/qqbar.py", line 4007, in interval raise TypeError("unable to convert {} to real interval".format(self)) TypeError: unable to convert 2.618826569900552?  0.2237500597843041?*I to real interval ********************************************************************** File "src/sage/rings/qqbar.py", line 3700, in sage.rings.qqbar.AlgebraicNumber_base.as_number_field_element Failed example: RR(elt) Exception raised: Traceback (most recent call last): File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 659, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 1070, in compile_and_execute exec(compiled, globs) File "<doctest sage.rings.qqbar.AlgebraicNumber_base.as_number_field_element[13]>", line 1, in <module> RR(elt) File "sage/structure/parent.pyx", line 921, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9679) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 271, in sage.structure.coerce_maps.NamedConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:5993) cdef Element e = method(C) File "sage/rings/number_field/number_field_element.pyx", line 1802, in sage.rings.number_field.number_field_element.NumberFieldElement._mpfr_ (build/cythonized/sage/rings/number_field/number_field_element.cpp:19625) return R(R.complex_field()(self)) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/rings/complex_field.py", line 381, in __call__ return Parent.__call__(self, x) File "sage/structure/parent.pyx", line 921, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9679) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 145, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4574) raise File "sage/structure/coerce_maps.pyx", line 140, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4442) return C._element_constructor(x) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/rings/complex_field.py", line 423, in _element_constructor_ return ComplexNumber(self, x) File "sage/rings/complex_number.pyx", line 200, in sage.rings.complex_number.ComplexNumber.__init__ (build/cythonized/sage/rings/complex_number.c:4535) rr = R(real) File "sage/structure/parent.pyx", line 921, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9679) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 271, in sage.structure.coerce_maps.NamedConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:5993) cdef Element e = method(C) File "sage/rings/number_field/number_field_element.pyx", line 1802, in sage.rings.number_field.number_field_element.NumberFieldElement._mpfr_ (build/cythonized/sage/rings/number_field/number_field_element.cpp:19625) return R(R.complex_field()(self)) File "/Users/mkoeppe/s/sage/sagerebasing/worktreeclean/local/lib/python2.7/sitepackages/sage/rings/complex_field.py", line 381, in __call__ return Parent.__call__(self, x) File "sage/structure/parent.pyx", line 921, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9679) return mor._call_(x) File "sage/structure/coerce_maps.pyx", line 145, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4574) raise File "sage/structure/coerce_maps.pyx", line 140, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4442) ..................... File "sage/categories/map.pyx", line 176, in sage.categories.map.Map.__copy__ (build/cythonized/sage/categories/map.c:4133) cdef Map out = Element.__copy__(self) File "sage/structure/element.pyx", line 605, in sage.structure.element.Element.__copy__ (build/cythonized/sage/structure/element.c:5273) D = self.__dict__ File "sage/structure/element.pyx", line 493, in sage.structure.element.Element.__getattr__ (build/cythonized/sage/structure/element.c:4550) return self.getattr_from_category(name) File "sage/structure/element.pyx", line 506, in sage.structure.element.Element.getattr_from_category (build/cythonized/sage/structure/element.c:4659) return getattr_from_other_class(self, cls, name) File "sage/cpython/getattr.pyx", line 386, in sage.cpython.getattr.getattr_from_other_class (build/cythonized/sage/cpython/getattr.c:2428) if isinstance(self, cls): RuntimeError: maximum recursion depth exceeded while calling a Python object ********************************************************************** 1 item had failures: 3 of 21 in sage.rings.qqbar.AlgebraicNumber_base.as_number_field_element [1438 tests, 3 failures, 14.69 s]  sage t src/sage/rings/qqbar.py # 3 doctests failed 
For the infinite recursion in the last failure, perhaps could someone experienced with the Sage coercion system help?
comment:37 followup: ↓ 38 Changed 2 years ago by
So I think I know what is going wrong. The issue is that there is no coercion from RIF > RR
; it goes the other way around. So with these changes, I can avoid the infinite recursion with RR(elt)
, but I get other failures:

src/sage/rings/qqbar.py
diff git a/src/sage/rings/qqbar.py b/src/sage/rings/qqbar.py index b32aa1a..7453d89 100644
a b def number_field_elements_from_algebraics(numbers, minimal=False, same_field=False, embedded=False, prec=53): 2197 2197 2198 2198 # Make the numbers have a real root 2199 2199 real_numbers = [] 2200 from sage.rings.real_mpfr import RealField 2200 2201 for v in numbers: 2201 2202 if v._exact_field().is_complex() and real_case: 2202 2203 # the number comes from a complex algebraic number field 2203 embedded_rt = v.interval_fast(Real IntervalField(prec))2204 embedded_rt = v.interval_fast(RealField(prec)) 2204 2205 root = ANRoot(v.minpoly(), embedded_rt) 2205 2206 real_nf = NumberField(v.minpoly(),'a') 2206 2207 new_ef = AlgebraicGenerator(real_nf, root) … … def number_field_elements_from_algebraics(numbers, minimal=False, same_field=False, embedded=False, prec=53): 2223 2224 if fld is not QQ and embedded: 2224 2225 assert real_case 2225 2226 exact_generator = hom(fld.gen(0)) 2226 embedding_field = Real IntervalField(prec)2227 embedding_field = RealField(prec) 2227 2228 embedded_gen = embedding_field(exact_generator) 2228 2229 embedded_field = NumberField(fld.defining_polynomial(),fld.variable_name(),embedding=embedded_gen) 2229 2230 2230 inter_hom = fld.hom([embedded_field.gen(0)]) 2231 2231 nums = map(inter_hom, nums) 2232 2232 hom = embedded_field.hom([gen.root_as_algebraic()])
Note that with your current branch, the replacement of the RR(elt)
by RIF(elt)
works.
The reason you get the infinite recursion is somewhere in the number field coercion (conversion?)( code, it is trying to construct a complex number, which then tries to create the RR
, but it cannot do that, so it tried to construct a number field element, and hence the cycle. Or something like that. The reason it normally doesn't cycle is because it separately knows how to get the the information it needs (if it even falls into this cycle). I don't completely understand the mechanism, but this is what the traceback is suggesting to me.
comment:38 in reply to: ↑ 37 Changed 2 years ago by
Replying to tscrim:
So I think I know what is going wrong. The issue is that there is no coercion from
RIF > RR
; it goes the other way around.
Note that #15114 is for removing this coercion, but is currently blocked by more or less the same issues with comparisons between elements of different parents that need to be solved for the Python 3 transition.
comment:39 Changed 19 months ago by
 Description modified (diff)
comment:40 Changed 19 months ago by
Just as a summary here are things that need to be done (maybe some of them are already done):
 you may want to patch
as_number_field_element()
too,  don't forget to add tests covering the case where some or all of the elements are rationals or elements of quadratic extensions (rationals, quadratic number field elements and higherdegree number field elements don't have exactly the same interface...)
 Fix doctest in File "qqbar.py", line 2098, in sage.rings.qqbar.number_field_elements_from_algebraics
as_number_field_element
needs to pass on its new arguments, and corresponding doctests should be added. Fix Comment 34 and Comment 36.
comment:41 Changed 18 months ago by
 Commit changed from 2eb332053089cccf9266850d2fbd964a3c24b9fe to 5793bcfce6f66f5740fce05c4d89c1587eaee872
Branch pushed to git repo; I updated commit sha1. New commits:
5793bcf  Merge branch 'public/algnum_to_numberfield' of trac.sagemath.org:sage into 20181

comment:42 Changed 18 months ago by
 Milestone changed from sage8.4 to sage8.8
comment:43 Changed 18 months ago by
While rereading the docstring, I realized that it is quite confusing. Especially the following lines:
2154 Note that for the first example, where \sage does not realize that 2155 the number is real, we get a homomorphism to ``QQbar``; but with 2156 ``minimal=True``, we get a homomorphism to ``AA``. If we specify 2157 both ``minimal=True`` and ``same_field=True``, we get a second 2158 degree extension (minimal) that maps back to ``QQbar``.
comment:44 Changed 18 months ago by
 Commit changed from 5793bcfce6f66f5740fce05c4d89c1587eaee872 to cb63fe51cda3ac27a67c297e9a05289b2911c865
Branch pushed to git repo; I updated commit sha1. New commits:
cb63fe5  Added some doctest for embedded case

comment:45 Changed 18 months ago by
 Commit changed from cb63fe51cda3ac27a67c297e9a05289b2911c865 to b913dcc0bc20886e637ffe2e05a88907f8eccdf2
Branch pushed to git repo; I updated commit sha1. New commits:
b913dcc  Fixed usage of RealField and docstring

comment:46 Changed 18 months ago by
 Commit changed from b913dcc0bc20886e637ffe2e05a88907f8eccdf2 to b153ef700dd3bab9966283e69ae24afc3bbf2245
Branch pushed to git repo; I updated commit sha1. New commits:
b153ef7  fixed docstring

comment:47 Changed 18 months ago by
 Commit changed from b153ef700dd3bab9966283e69ae24afc3bbf2245 to 61e4df9fabf9e06a21d75eb6320ab3179bc28096
Branch pushed to git repo; I updated commit sha1. New commits:
61e4df9  Fix docstring even more

comment:48 Changed 18 months ago by
 Status changed from needs_work to needs_review
I deleted an old comment that should have been deleted in 2010 from ticket #9343.
It now looks ready for review.
comment:49 Changed 18 months ago by
 Commit changed from 61e4df9fabf9e06a21d75eb6320ab3179bc28096 to 9255f2c89588c79de496660c8c9030f087b5ebfd
Branch pushed to git repo; I updated commit sha1. New commits:
9255f2c  pyflakes

comment:50 Changed 18 months ago by
Looks good to me and works well within #25097. But it would be good if another reviewer could take a look as well.
comment:51 Changed 18 months ago by
It would be great is a semifulltime number theorist looks into this...
comment:52 Changed 18 months ago by
I almost never use embedded number fields (instead I make explicit embeddings into RR or CC as needed), but this all looks reasonable and there is a lot of new documentation and examples, which is good. So OK from me.
comment:53 Changed 18 months ago by
 Reviewers changed from Matthias Koeppe to Matthias Koeppe, John Cremona
 Status changed from needs_review to positive_review
comment:54 Changed 17 months ago by
 Branch changed from public/algnum_to_numberfield to 9255f2c89588c79de496660c8c9030f087b5ebfd
 Resolution set to fixed
 Status changed from positive_review to closed
comment:55 Changed 17 months ago by
 Commit 9255f2c89588c79de496660c8c9030f087b5ebfd deleted
you used "map" in a way that is not compatible with py3, please review the fix in #27767
While looking at related tickets, I noticed the very old ticket #5355, which should be closed I think.