# AlgebraicNumber should be a domain element? #14389

Open
opened this Issue Mar 5, 2018 · 6 comments

Projects
None yet
2 participants
Contributor

### skirpichev commented Mar 5, 2018

 I think, this was a mistake to make it an Expr. The last class - represents very rough idea of mathematical expression, but AlgebraicNumber - much more structure-rich representation for elements of some special algebraic number field. This also can't be a canonical way to represent algebraic numbers, because such a representation will not be unique (e.g. sqrt(2) is an element of Q(sqrt(2)) and Q(sqrt(2) + sqrt(3))). Probably, this should be a subclass of the DomainElement (ANP will be deprecated), much like ModularInteger or PolyElement. And it shouldn't be in a hierarchy, derived from Basic.
Member

### jksuom commented Mar 5, 2018

 It seems that `AlgebraicNumber` is not much used internally in SymPy. It appears in `field_isomorphism` but could probably be replaced by some non-Basic class. `AlgebraicField.to_sympy` creates an `AlgebraicNumber`, and it may be harder to find a replacement. Expressions in terms of a generator may become complicated and inconvenient. Canonical representation of algebraic numbers seems a hard problem to me. I think that working in a fixed number field with a chosen primitive element cannot be avoided. In such a field the representation is then canonical.
Contributor

### skirpichev commented Mar 5, 2018

 Expressions in terms of a generator may become complicated and inconvenient. They might be also much simpler after expansion, e.g. ``````In [12]: to_number_field(sqrt(2), sqrt(2) + sqrt(3)).as_expr() Out[12]: ___ ╲╱ 2 In [13]: to_number_field(sqrt(2), sqrt(2) + sqrt(3)).as_expr(y).subs(y, sqrt(2) + sqrt(3)) Out[13]: 3 ___ ___ ⎛ ___ ___⎞ 9⋅╲╱ 3 9⋅╲╱ 2 ⎝╲╱ 2 + ╲╱ 3 ⎠ - ─────── - ─────── + ──────────────── 2 2 2 `````` Canonical representation of algebraic numbers seems a hard problem to me. RootOf maybe? After all, that's a definition of the field of all algebraic numbers... The hard part is arithmetics in this field. I think that working in a some finite extension of QQ is always unavoidable internally (to do arithmetics efficiently). fixed number field with a chosen primitive element cannot be avoided. In such a field the representation is then canonical. As a side note, please keep in mind that fixing a primitive element somewhat arbitrary. We always could choose a different basis for same field (and representation of elements will be different, by the way). Perhaps, the AlgebraicField class design must reflect this fact. I.e. for `QQ(a, b)` we should keep both algebraic numbers that specify this extension. Some algorithms may also work without the need to generate a primitive element.
Member

### jksuom commented Mar 5, 2018

 for QQ(a, b) we should keep both algebraic numbers that specify this extension. Actually, the arguments are saved (`orig_ext`) but currently not used. Maybe `to_number_field` could be extended to return an expression of the primitive element in terms of those. Also, we could think of doing some preprocessing on the arguments (`1/√2` -> `√2`) before saving them.
Contributor

### skirpichev commented Mar 12, 2018

 Maybe to_number_field could be extended to return an expression of the primitive element in terms of those. Essentially this `primitive_element()` does, at least with `ex=True`.
Contributor

### skirpichev commented Mar 14, 2018

 diofant/diofant#619 address this issue. But perhaps, from beginning there should be more sophisticated class hierarchy, i.e. separate class for real field (elements must support comparison ops).
Member

### jksuom commented Mar 15, 2018

 separate class for real field (elements must support comparison ops). I agree that a subclass, `RealAlgebraicField`, is much needed. I did think of it in connection with CAD a few years ago though I forget the details now.

### skirpichev added a commit to skirpichev/diofant that referenced this issue Mar 15, 2018

``` domains: new class to represent AlgebraicField elements ```
```Replaces ANP class from polyclasses.py

Closes sympy/sympy#14389```
``` 652f147 ```