#21161 closed enhancement (fixed)
repr of NumberFields (the parents) should indicate its embedding if there is one
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.8 |
Component: | number fields | Keywords: | number field |
Cc: | vdelecroix, jipilab, cremona, tscrim, mmezzarobba, jdemeyer | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | Jean-Philippe Labbé |
Report Upstream: | N/A | Work issues: | |
Branch: | d404006 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
As discussed in #21105, number fields with coercion embeddings, in particular with real embeddings, behave quite differently from those without - but there's no indication of embeddings in the print representation:
sage: K.<a> = NumberField(x^2 - 2) sage: a.parent() Number Field in a with defining polynomial x^2 - 2 sage: K.<sqrt2> = NumberField(x^2 - 2, embedding=1.4) sage: sqrt2.parent() Number Field in sqrt2 with defining polynomial x^2 - 2
This ticket changes the print representation when there is an embedding as follows.
Number Field in a with defining polynomial x^13 - 2/3*x + 3 with a = -1.106745229567614?
This also works well for more complicated situations such as this one:
sage: K.<a> = NumberField(x^4 - 3); K Number Field in a with defining polynomial x^4 - 3 sage: H.<b>, from_H = K.subfield(a^2) sage: H Number Field in b with defining polynomial x^2 - 3 with b = a^2
Change History (41)
comment:1 Changed 5 years ago by
- Description modified (diff)
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
Or possibly
Real Number Field in sqrt2=1.41421356237309? with defining polynomial x^2 - 2
comment:4 follow-up: ↓ 5 Changed 5 years ago by
AlgebraicGenerator
(from qqbar
) uses
Number Field in a with defining polynomial y^2 - y - 1 with a in 1.618033988749895?
comment:5 in reply to: ↑ 4 Changed 5 years ago by
Replying to mkoeppe:
AlgebraicGenerator
(fromqqbar
) usesNumber Field in a with defining polynomial y^2 - y - 1 with a in 1.618033988749895?
Which is not that bad except the a in 1.618033988749895?
. Would make more sense with a in [1.61, 1.62]
or a = 1.618033988749895?
.
comment:6 Changed 3 years ago by
- Cc jipilab added
comment:7 Changed 2 years ago by
This should work for number fields with a real embedding as well as a complex one. The interval notation is painful for the latter, so "with a=..." would probably work better. Also good if we end up with number fields with a p-adic embedding specified:
K.<a>=NumberField(x^2+1,embedding=pAdicField(5)(-1).sqrt())
comment:8 Changed 23 months ago by
- Branch set to u/mkoeppe/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one
comment:9 Changed 23 months ago by
- Commit set to d40c9e6d41409883ff84f007f3ecbe8dbfc45fab
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d40c9e6 | NumberField_generic._repr_: Print embedding if there is one
|
comment:10 Changed 23 months ago by
- Commit changed from d40c9e6d41409883ff84f007f3ecbe8dbfc45fab to ba3b35d27961bf0f28b993626ce3cc91f0cdae6b
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ba3b35d | NumberField_generic._repr_: Print embedding if there is one
|
comment:11 Changed 23 months ago by
In this patch I am using a format with with a = ...
now (as suggested by nbruin), which works well also for the complex case:
Number Field in a with defining polynomial x^13 - 2/3*x + 3 with a = -1.106745229567614?
and for more complicated situations such as this one:
sage: K.<a> = NumberField(x^4 - 3); K Number Field in a with defining polynomial x^4 - 3 sage: H.<b>, from_H = K.subfield(a^2) sage: H Number Field in b with defining polynomial x^2 - 3 with b = a^2
The output for the following looks a bit strange because printing goes through sage.rings.real_lazy.LazyBinop
:
sage: QuadraticField(-1, 'a') Number Field in a with defining polynomial x^2 + 1 with a = 1*I
Should we special case this?
comment:12 Changed 23 months ago by
- Commit changed from ba3b35d27961bf0f28b993626ce3cc91f0cdae6b to ed3aab0e2c017c0841a982f91681ec5f51801fd9
Branch pushed to git repo; I updated commit sha1. New commits:
ed3aab0 | Update doctest outputs
|
comment:13 Changed 23 months ago by
- Milestone changed from sage-7.4 to sage-8.8
- Status changed from new to needs_review
comment:14 Changed 23 months ago by
- Branch changed from u/mkoeppe/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one to public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one
- Commit changed from ed3aab0e2c017c0841a982f91681ec5f51801fd9 to 875731c245916cfd34647a517e83a81968cc1b15
- Keywords number field added
- Reviewers set to Jean-Philippe Labbé
- Status changed from needs_review to positive_review
The patchbot gave 2 pyflakes warnings, I repaired one of them, the other one I can not make sure that it does not have side effects.
Otherwise, the patchbot is happy and the resolution looks reasonable. I set it as positive review, if you agree @mkoeppe.
New commits:
875731c | fixed one pyflakes
|
comment:16 Changed 23 months ago by
- Commit changed from 875731c245916cfd34647a517e83a81968cc1b15 to 895b9624338a94aaf7dbbdcc1dbe76ff49e90dd9
Branch pushed to git repo; I updated commit sha1. New commits:
895b962 | Made doctests pass
|
comment:17 Changed 23 months ago by
- Status changed from needs_work to needs_review
Let's see if the bot finds more. I hope I did not forget any...
comment:19 Changed 23 months ago by
- Commit changed from 895b9624338a94aaf7dbbdcc1dbe76ff49e90dd9 to 353b01bfd8bd504655a9b43577ef224f4bb60b73
Branch pushed to git repo; I updated commit sha1. New commits:
353b01b | Make more doctests pass
|
comment:20 Changed 23 months ago by
- Status changed from needs_work to needs_review
comment:21 Changed 23 months ago by
- Cc cremona tscrim added
comment:22 Changed 23 months ago by
- Commit changed from 353b01bfd8bd504655a9b43577ef224f4bb60b73 to 2ee8fccead494533ebe1c50cc35a08ae54d8d499
Branch pushed to git repo; I updated commit sha1. New commits:
2ee8fcc | Make more doctests pass
|
comment:23 Changed 23 months ago by
sage -t --long src/sage/modular/local_comp/smoothchar.py ********************************************************************** File "src/sage/modular/local_comp/smoothchar.py", line 461, in sage.modular.local_comp.smoothchar.SmoothCharacterGroupGeneric._coerce_map_from_ Failed example: G.coerce(GK.character(0, [4])) Exception raised: Traceback (most recent call last): File "/home/sage-patchbot/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/sage-patchbot/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute exec(compiled, globs) File "<doctest sage.modular.local_comp.smoothchar.SmoothCharacterGroupGeneric._coerce_map_from_[7]>", line 1, in <module> G.coerce(GK.character(Integer(0), [Integer(4)])) File "sage/structure/parent.pyx", line 1114, in sage.structure.parent.Parent.coerce (build/cythonized/sage/structure/parent.c:10541) cpdef coerce(self, x): File "sage/structure/parent.pyx", line 1144, in sage.structure.parent.Parent.coerce (build/cythonized/sage/structure/parent.c:10470) raise TypeError(_LazyString(_lazy_format, ("no canonical coercion from %s to %s", parent(x), self), {})) TypeError: no canonical coercion from Group of smooth characters of Q_3* with values in Number Field in i with defining polynomial x^2 + 1 with i = 1*I to Group of smooth characters of Q_3* with values in Rational Field ********************************************************************** 1 item had failures: 1 of 14 in sage.modular.local_comp.smoothchar.SmoothCharacterGroupGeneric._coerce_map_from_ [303 tests, 1 failure, 2.58 s] sage -t --long src/sage/tests/books/computational-mathematics-with-sagemath/polynomes_doctest.py ********************************************************************** File "src/sage/tests/books/computational-mathematics-with-sagemath/polynomes_doctest.py", line 177, in sage.tests.books.computational-mathematics-with-sagemath.polynomes_doctest Failed example: for A in [QQ, ComplexField(16), GF(5), QQ[sqrt(2)]]: print(str(A) + ":") print(A['x'](p).factor()) Expected: Rational Field: (54) * (x + 1/3)^2 * (x^2 - 2) Complex Field with 16 bits of precision: (54.00) * (x - 1.414) * (x + 0.3333)^2 * (x + 1.414) Finite Field of size 5: (4) * (x + 2)^2 * (x^2 + 3) Number Field in sqrt2 with defining polynomial x^2 - 2: (54) * (x - sqrt2) * (x + sqrt2) * (x + 1/3)^2 Got: Rational Field: (54) * (x + 1/3)^2 * (x^2 - 2) Complex Field with 16 bits of precision: (54.00) * (x - 1.414) * (x + 0.3333)^2 * (x + 1.414) Finite Field of size 5: (4) * (x + 2)^2 * (x^2 + 3) Number Field in sqrt2 with defining polynomial x^2 - 2 with sqrt2 = 1.414213562373095?: (54) * (x - sqrt2) * (x + sqrt2) * (x + 1/3)^2 ********************************************************************** 1 item had failures: 1 of 111 in sage.tests.books.computational-mathematics-with-sagemath.polynomes_doctest [110 tests, 1 failure, 0.96 s] ---------------------------------------------------------------------- sage -t --long src/sage/modular/local_comp/smoothchar.py # 1 doctest failed sage -t --long src/sage/tests/books/computational-mathematics-with-sagemath/polynomes_doctest.py # 1 doctest failed ----------------------------------------------------------------------
The first one seems to be taken care of but not the second one.
comment:24 Changed 23 months ago by
- Commit changed from 2ee8fccead494533ebe1c50cc35a08ae54d8d499 to 9ce74e38ac34f716121c75991c02528e95f771d1
Branch pushed to git repo; I updated commit sha1. New commits:
9ce74e3 | Doctest fix in src/sage/tests/books/computational-mathematics-with-sagemath/polynomes_doctest.py
|
comment:25 Changed 23 months ago by
- Commit changed from 9ce74e38ac34f716121c75991c02528e95f771d1 to 76c384e5399ab8ab36920f42f4715c0225b3a273
Branch pushed to git repo; I updated commit sha1. New commits:
76c384e | Fix up doctest after change
|
comment:26 Changed 23 months ago by
Fixed these two.
comment:27 Changed 23 months ago by
- Description modified (diff)
comment:28 Changed 23 months ago by
- Cc mmezzarobba added
comment:29 Changed 22 months ago by
- Status changed from needs_review to needs_work
comment:30 Changed 22 months ago by
- Commit changed from 76c384e5399ab8ab36920f42f4715c0225b3a273 to c4c0088933e236cab5295e1f8f79c0d9d6a0ea8c
Branch pushed to git repo; I updated commit sha1. New commits:
c4c0088 | Merge tag '8.8.beta5' into t/21161/public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one
|
comment:31 Changed 22 months ago by
- Cc jdemeyer added
- Status changed from needs_work to needs_review
comment:32 Changed 22 months ago by
- Status changed from needs_review to needs_work
There are only 6 failing doctests. That's not bad!
---------------------------------------------------------------------- sage -t --long src/sage/rings/qqbar.py # 5 doctests failed sage -t --long src/sage/rings/number_field/order.py # 1 doctest failed ----------------------------------------------------------------------
comment:33 Changed 22 months ago by
- Commit changed from c4c0088933e236cab5295e1f8f79c0d9d6a0ea8c to d404006b1ecdbeb5c4a70b5f43301541ae3c50f7
comment:34 Changed 22 months ago by
- Status changed from needs_work to needs_review
comment:35 Changed 22 months ago by
- Status changed from needs_review to positive_review
Looks good to me. The tests seem to pass on the latest beta and the pyflakes errors are not regressions. I set this to positive review.
comment:36 Changed 22 months ago by
- Branch changed from public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one to d404006b1ecdbeb5c4a70b5f43301541ae3c50f7
- Resolution set to fixed
- Status changed from positive_review to closed
comment:37 follow-up: ↓ 38 Changed 21 months ago by
- Commit d404006b1ecdbeb5c4a70b5f43301541ae3c50f7 deleted
This ticket is probably causing (randomly) infinite loops on several patchbots.
For example, see
EDIT: the problematic doctest is
File "src/sage/structure/parent.pyx", line 1734, in sage.structure.parent.Parent.hom.register_embedding Failed example: K.coerce_embedding()(a) Exception raised: ... RuntimeError: maximum recursion depth exceeded while calling a Python object
comment:38 in reply to: ↑ 37 Changed 21 months ago by
Replying to chapoton:
This ticket is probably causing (randomly) infinite loops on several patchbots.
For example, see
EDIT: the problematic doctest is
File "src/sage/structure/parent.pyx", line 1734, in sage.structure.parent.Parent.hom.register_embedding Failed example: K.coerce_embedding()(a) Exception raised: ... RuntimeError: maximum recursion depth exceeded while calling a Python object
The source code that changed is
-
src/sage/rings/number_field/number_field.py
a b class NumberField_generic(WithEqualityById, number_field_base.NumberField): 3087 3089 """ 3088 return "Number Field in %s with defining polynomial %s"%( 3089 self.variable_name(), self.polynomial()) 3090 result = "Number Field in {} with defining polynomial {}".format(self.variable_name(), self.polynomial()) 3091 gen = self.gen_embedding() 3092 if gen is not None: 3093 result += " with {} = {}".format(self.variable_name(), gen) 3094 return result
The rest of the diff consist of adapting the doctests. Where could the infinite loop come from? Hmm.
comment:39 Changed 21 months ago by
It appears that using the embedding in the repr is a very bad idea...
Even when this problematic doctest works, calling
K.coerce_embedding()
after this doctest makes sage crash.. And the culprit are really those 2 "innocent" added lines..
comment:40 Changed 21 months ago by
I have made #28036 to revert the changes made here.
comment:41 Changed 21 months ago by
Maybe changing this line would be a way out:
return copy(self._embedding) # It might be overkill to make a copy here
Shorter sentence would be better.
Ideally (as above), the
X
innear X
should have the form of an interval[1.41,1.42]
that specifies uniquely the root.