#20181 closed enhancement (fixed)
number_field_elements_from_algebraics should create embedded number field elements
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-8.8 |
Component: | number fields | Keywords: | IMA-PolyGeom |
Cc: | vbraun, gagern, vdelecroix, tmonteil, jj, sstarosta, jipilab, moritz, yzh, cremona, tscrim | Merged in: | |
Authors: | Jean-Philippe 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 follow-up on #17830 and #18819.
#17830 made this possible:
sage: x = polygen(ZZ) sage: K.<cbrt2> = NumberField(x^3 - 2, embedding=AA.polynomial_root(x^3-2, RIF(0,3))) sage: 6064/4813 < cbrt2 < 90325/71691 True
But if one uses number_field_elements_from_algebraics
, one gets only non-embedded number field elements:
sage: x = polygen(ZZ) sage: K, cbrt2, hom = number_field_elements_from_algebraics(AA.polynomial_root(x^3-2, 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^3-2, 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 3 years ago by
- Cc jipilab moritz added
comment:4 Changed 3 years ago by
- Cc yzh added
comment:5 Changed 3 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 3 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 3 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.67361737988404e-18*I, 231.039466852489 - 1.04083408558608e-17*I, 156.026311234993 - 8.67361737988404e-18*I, 79.0131556174964 - 4.33680868994202e-18*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 3 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 3 years ago by
It is now possible to get this:
sage: UCF = UniversalCyclotomicField() sage: E = UCF.gen(5) sage: L.<b> = NumberField(x^2-189*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 3 years ago by
- Status changed from needs_info to needs_review
comment:11 Changed 3 years ago by
comment:12 follow-up: ↓ 17 Changed 3 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) <ipython-input-8-74370e91d982> in <module>() ----> 1 res[Integer(2)](my_nums[Integer(0)]) /Users/mkoeppe/s/sage/some-sage-install-prefix/lib/python2.7/site-packages/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 follow-up: ↓ 14 Changed 3 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 higher-degree number field elements don't have exactly the same interface...)
comment:14 in reply to: ↑ 13 Changed 3 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 higher-degree 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 3 years ago by
- Status changed from needs_review to needs_work
comment:16 Changed 3 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^3-2, 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 3 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) <ipython-input-8-74370e91d982> in <module>() ----> 1 res[Integer(2)](my_nums[Integer(0)]) /Users/mkoeppe/s/sage/some-sage-install-prefix/lib/python2.7/site-packages/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 3 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 3 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 3 years ago by
- Status changed from needs_work to needs_review
comment:21 Changed 3 years ago by
- Keywords IMA-PolyGeom added
comment:22 Changed 3 years ago by
- Milestone changed from sage-7.1 to sage-8.3
comment:23 follow-up: ↓ 33 Changed 3 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 3 years ago by
You should just go on and fix the doctest, I think.
comment:25 Changed 3 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 remote-tracking 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 remote-tracking 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 sage-8.3 to sage-8.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 re-review?
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 follow-up: ↓ 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 2018-09-27-16-59-10-65572ec7. 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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 659, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 659, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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/sage-rebasing/worktree-clean/local/lib/python2.7/site-packages/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 follow-up: ↓ 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 2 years ago by
- Description modified (diff)
comment:40 Changed 2 years 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 higher-degree 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 23 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 23 months ago by
- Milestone changed from sage-8.4 to sage-8.8
comment:43 Changed 23 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 23 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 23 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 23 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 23 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 23 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 23 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 22 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 22 months ago by
It would be great is a semi-full-time number theorist looks into this...
comment:52 Changed 22 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 22 months ago by
- Reviewers changed from Matthias Koeppe to Matthias Koeppe, John Cremona
- Status changed from needs_review to positive_review
comment:54 Changed 22 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 22 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.