Opened 5 years ago

Closed 3 years ago

# repr of NumberFields (the parents) should indicate its embedding if there is one

Reported by: Owned by: mkoeppe major sage-8.8 number fields number field vdelecroix, jipilab, cremona, tscrim, mmezzarobba, jdemeyer Matthias Koeppe Jean-Philippe Labbé N/A d404006

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
```

### comment:1 Changed 5 years ago by mkoeppe

• Description modified (diff)

### comment:2 Changed 5 years ago by vdelecroix

Shorter sentence would be better.

```Real Number Field in sqrt2 as the root of x^2 - 2 in [1.41, 1.42]
```

Ideally (as above), the `X` in `near X` should have the form of an interval `[1.41,1.42]` that specifies uniquely the root.

Last edited 5 years ago by vdelecroix (previous) (diff)

### comment:3 Changed 5 years ago by vdelecroix

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 mkoeppe

`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 vdelecroix

`AlgebraicGenerator` (from `qqbar`) uses

```   Number 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:7 Changed 3 years ago by nbruin

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())
```
Last edited 3 years ago by nbruin (previous) (diff)

### comment:8 Changed 3 years ago by mkoeppe

• Branch set to u/mkoeppe/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one

### comment:9 Changed 3 years ago by git

• 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 3 years ago by git

• 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 3 years ago by mkoeppe

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 3 years ago by git

• Commit changed from ba3b35d27961bf0f28b993626ce3cc91f0cdae6b to ed3aab0e2c017c0841a982f91681ec5f51801fd9

Branch pushed to git repo; I updated commit sha1. New commits:

 ​ed3aab0 `Update doctest outputs`

### comment:13 Changed 3 years ago by mkoeppe

• Authors set to Matthias Koeppe
• Milestone changed from sage-7.4 to sage-8.8
• Status changed from new to needs_review

### comment:14 Changed 3 years ago by jipilab

• 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
• 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:15 Changed 3 years ago by jipilab

• Status changed from positive_review to needs_work

Whoooops!

### comment:16 Changed 3 years ago by git

• Commit changed from 875731c245916cfd34647a517e83a81968cc1b15 to 895b9624338a94aaf7dbbdcc1dbe76ff49e90dd9

Branch pushed to git repo; I updated commit sha1. New commits:

 ​895b962 `Made doctests pass`

### comment:17 Changed 3 years ago by jipilab

• Status changed from needs_work to needs_review

Let's see if the bot finds more. I hope I did not forget any...

### comment:18 Changed 3 years ago by jipilab

• Status changed from needs_review to needs_work

Failing doctests

### comment:19 Changed 3 years ago by git

• 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 3 years ago by mkoeppe

• Status changed from needs_work to needs_review

### comment:22 Changed 3 years ago by git

• 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 3 years ago by jipilab

```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 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 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
----------------------------------------------------------------------
```
Version 0, edited 3 years ago by jipilab (next)

### comment:24 Changed 3 years ago by git

• 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 3 years ago by git

• Commit changed from 9ce74e38ac34f716121c75991c02528e95f771d1 to 76c384e5399ab8ab36920f42f4715c0225b3a273

Branch pushed to git repo; I updated commit sha1. New commits:

 ​76c384e `Fix up doctest after change`

Fixed these two.

### comment:27 Changed 3 years ago by mkoeppe

• Description modified (diff)

### comment:29 Changed 3 years ago by jipilab

• Status changed from needs_review to needs_work

### comment:30 Changed 3 years ago by git

• 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 3 years ago by mkoeppe

• Status changed from needs_work to needs_review

### comment:32 Changed 3 years ago by jipilab

• 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 3 years ago by git

• Commit changed from c4c0088933e236cab5295e1f8f79c0d9d6a0ea8c to d404006b1ecdbeb5c4a70b5f43301541ae3c50f7

Branch pushed to git repo; I updated commit sha1. New commits:

 ​c1e1514 `Merge tag '8.8.beta6' into t/21161/public/repr_of_numberfields__the_parents__should_indicate_its_embedding_if_there_is_one` ​d404006 `Fix doctests`

### comment:34 Changed 3 years ago by mkoeppe

• Status changed from needs_work to needs_review

### comment:35 Changed 3 years ago by jipilab

• 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 3 years ago by vbraun

• 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 2 years ago by chapoton

• 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
```
Last edited 2 years ago by chapoton (previous) (diff)

### comment:38 in reply to: ↑ 37 Changed 2 years ago by jipilab

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 class NumberField_generic(WithEqualityById, number_field_base.NumberField): """ return "Number Field in %s with defining polynomial %s"%( self.variable_name(), self.polynomial()) result = "Number Field in {} with defining polynomial {}".format(self.variable_name(), self.polynomial()) gen = self.gen_embedding() if gen is not None: result += " with {} = {}".format(self.variable_name(), gen) return result

The rest of the diff consist of adapting the doctests. Where could the infinite loop come from? Hmm.

### comment:39 Changed 2 years ago by chapoton

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

Last edited 2 years ago by chapoton (previous) (diff)

### comment:40 Changed 2 years ago by chapoton

```return copy(self._embedding) # It might be overkill to make a copy here