Skip to content

Commit

Permalink
Removed old, Expr-derived AlgebraicNumber class
Browse files Browse the repository at this point in the history
There are little regression in supporting simplifications of
algebraic numbers.  Former AlgebraicNumber._eval_simplify()
method was naive and not very powerful.
  • Loading branch information
skirpichev committed Jun 14, 2018
1 parent bebcadf commit cb6e730
Show file tree
Hide file tree
Showing 25 changed files with 119 additions and 546 deletions.
2 changes: 1 addition & 1 deletion diofant/core/__init__.py
Expand Up @@ -9,7 +9,7 @@
from .symbol import Symbol, Wild, Dummy, symbols, var # noqa: F401
from .numbers import (Number, Float, Rational, Integer, # noqa: F401
NumberSymbol, igcd, ilcm, seterr, E, I, nan, oo,
pi, zoo, AlgebraicNumber, comp, mod_inverse)
pi, zoo, comp, mod_inverse)
from .power import Pow, integer_nthroot # noqa: F401
from .mul import Mul, prod # noqa: F401
from .add import Add # noqa: F401
Expand Down
4 changes: 0 additions & 4 deletions diofant/core/add.py
Expand Up @@ -82,10 +82,6 @@ def flatten(cls, seq):
o # XXX "peephole" optimization, http://bugs.python.org/issue2506
continue

elif o.is_AlgebraicNumber:
coeff += o
continue

elif o is zoo:
if coeff.is_finite is False:
# we know for sure the result will be nan
Expand Down
1 change: 0 additions & 1 deletion diofant/core/basic.py
Expand Up @@ -36,7 +36,6 @@ class Basic(object):
is_Derivative = False
is_Piecewise = False
is_Poly = False
is_AlgebraicNumber = False
is_Relational = False
is_Equality = False
is_Boolean = False
Expand Down
7 changes: 0 additions & 7 deletions diofant/core/mul.py
Expand Up @@ -19,7 +19,6 @@ class NC_Marker:
is_Order = False
is_Mul = False
is_Number = False
is_AlgebraicNumber = False
is_Poly = False

is_commutative = False
Expand Down Expand Up @@ -251,15 +250,9 @@ def flatten(cls, seq):
if coeff is nan:
# we know for sure the result will be nan
return [nan], [], None
elif coeff.is_AlgebraicNumber:
coeff *= o
o # XXX "peephole" optimization, http://bugs.python.org/issue2506
continue

elif o.is_AlgebraicNumber:
coeff *= o
continue

elif o is zoo:
if not coeff:
# 0 * zoo = NaN
Expand Down
180 changes: 3 additions & 177 deletions diofant/core/numbers.py
Expand Up @@ -1184,7 +1184,7 @@ def __neg__(self):
def __add__(self, other):
if isinstance(other, Rational):
return Rational(self.p*other.q + self.q*other.p, self.q*other.q)
elif isinstance(other, (Float, AlgebraicNumber)):
elif isinstance(other, Float):
return other + self
else:
return Number.__add__(self, other)
Expand All @@ -1193,7 +1193,7 @@ def __add__(self, other):
def __sub__(self, other):
if isinstance(other, Rational):
return Rational(self.p*other.q - self.q*other.p, self.q*other.q)
elif isinstance(other, (Float, AlgebraicNumber)):
elif isinstance(other, Float):
return -other + self
else:
return Number.__sub__(self, other)
Expand All @@ -1202,7 +1202,7 @@ def __sub__(self, other):
def __mul__(self, other):
if isinstance(other, Rational):
return Rational(self.p*other.p, self.q*other.q)
elif isinstance(other, (Float, AlgebraicNumber)):
elif isinstance(other, Float):
return other*self
else:
return Number.__mul__(self, other)
Expand Down Expand Up @@ -1638,180 +1638,6 @@ def __rfloordiv__(self, other):
converter[int] = Integer


class AlgebraicNumber(Expr):
r"""Class for algebraic numbers in Diofant.
Represents the algebraic number in the field `\mathbb Q[\theta]`
given by
.. math::
c_n \theta^n + c_{n-1} \theta^{n-1} + \dots + c_0
Parameters
==========
expr : Expr
A generator `\theta` for the algebraic number.
coeffs : tuple, optional
A tuple of rational coefficients `(c_n, c_{n-1},\dots,c_0)`.
The default is ``(1, 0)``.
See Also
========
diofant.polys.rootoftools.RootOf
"""

is_AlgebraicNumber = True
is_algebraic = True
is_number = True

def __new__(cls, expr, coeffs=(1, 0), **kwargs):
"""Construct a new algebraic number. """
from ..polys import Poly
from ..polys.polyclasses import DMP
from ..polys.numberfields import minimal_polynomial

expr = sympify(expr)

if isinstance(expr, (tuple, Tuple)):
minpoly, root = expr

if not minpoly.is_Poly:
minpoly = Poly(minpoly)
elif expr.is_AlgebraicNumber:
minpoly, root = expr.minpoly, expr.root
else:
if expr.free_symbols:
raise ValueError("Not a number: %s" % expr)

minpoly, root = minimal_polynomial(expr), expr
if kwargs.get('gen'):
minpoly = minpoly.replace(kwargs.get('gen'))

dom = minpoly.domain.field

if isinstance(coeffs, DMP):
rep = coeffs
else:
rep = DMP.from_diofant_list(sympify(coeffs), 0, dom)

if rep.degree() >= minpoly.degree():
rep = rep.rem(minpoly.rep)

coeffs = Tuple(*rep.all_coeffs())
args = root, coeffs

obj = Expr.__new__(cls, *args)

obj.rep = rep
obj.root = root
obj.minpoly = minpoly

return obj

@property
def free_symbols(self):
return set()

def _eval_power(self, expt):
if expt.is_Integer:
A = self.rep.domain.algebraic_field(self.root)
r = A(self.rep.rep)**int(expt)
return self.func(self, r.rep)

@_sympifyit('other', NotImplemented)
def __add__(self, other):
if other.is_Rational:
other = self.func(self, (other,))

if other.is_AlgebraicNumber:
if self.minpoly == other.minpoly and self.root == other.root:
return self.func(self, self.rep + other.rep)
else:
return Add(self, other, evaluate=False)
else:
return Number.__add__(self, other)

@_sympifyit('other', NotImplemented)
def __sub__(self, other):
if other.is_Rational:
other = self.func(self, (other,))

if other.is_AlgebraicNumber:
if self.minpoly == other.minpoly and self.root == other.root:
return self.func(self, self.rep - other.rep)
else:
return Add(self, -other, evaluate=False)
else:
return Number.__sub__(self, other)

@_sympifyit('other', NotImplemented)
def __mul__(self, other):
if other.is_Rational:
other = self.func(self, (other,))

if other.is_AlgebraicNumber:
if self.minpoly == other.minpoly and self.root == other.root:
return self.func(self, self.rep * other.rep)
else:
return Mul(self, other, evaluate=False)
else:
return Number.__mul__(self, other)

def _eval_evalf(self, prec):
return self.as_expr()._evalf(prec)

def as_poly(self, x=None):
"""Create a Poly instance from ``self``. """
from .symbol import Dummy
from ..polys import Poly, PurePoly
if x is not None:
return Poly.new(self.rep, x)
else:
return PurePoly.new(self.rep, Dummy('x'))

def as_expr(self, x=None):
"""Create a Basic expression from ``self``. """
return self.as_poly(x or self.root).as_expr().expand()

def coeffs(self):
"""Returns all Diofant coefficients of an algebraic number. """
return [self.rep.domain.to_expr(c) for c in self.rep.all_coeffs()]

def native_coeffs(self):
"""Returns all native coefficients of an algebraic number. """
return self.rep.all_coeffs()

def to_algebraic_integer(self):
"""Convert ``self`` to an algebraic integer. """
from ..polys import Poly
f = self.minpoly

if f.LC() == 1:
return self

coeff = f.LC()**(f.degree() - 1)
poly = f.compose(Poly(f.gen/f.LC()))

minpoly = poly*coeff
root = f.LC()*self.root

return AlgebraicNumber((minpoly, root), self.coeffs())

def _eval_simplify(self, ratio, measure):
from ..polys import RootOf, minimal_polynomial
from .symbol import Dummy

for r in [r for r in self.minpoly.all_roots() if r.func != RootOf]:
if minimal_polynomial(self.root - r)(Dummy()).is_Symbol:
# use the matching root if it's simpler
if measure(r) < ratio*measure(self.root):
return AlgebraicNumber(r)
return self


class RationalConstant(Rational):
"""
Abstract base class for rationals with specific behaviors
Expand Down
10 changes: 3 additions & 7 deletions diofant/core/tests/test_args.py
Expand Up @@ -8,8 +8,8 @@
import re
import warnings

from diofant import (ITE, Add, Adjoint, AlgebraicNumber, And, Atom, AtomicExpr,
Basic, BlockDiagMatrix, BlockMatrix, Complement, Contains,
from diofant import (ITE, Add, Adjoint, And, Atom, AtomicExpr, Basic,
BlockDiagMatrix, BlockMatrix, Complement, Contains,
DeferredVector, Derivative, Determinant, DiagonalMatrix,
DiagonalOf, Dict, Dummy, Equality, Equivalent, Expr,
FiniteSet, Float, FunctionMatrix, GrayCode, GreaterThan,
Expand All @@ -25,7 +25,7 @@
StrictLessThan, Subs, Subset, Sum, Symbol,
SymmetricDifference, Trace, Transpose, Tuple, Unequality,
Union, Wild, WildFunction, Xor, ZeroMatrix, divisor_sigma,
false, mobius, oo, sin, sqrt, symbols, totient, true)
false, mobius, oo, sin, symbols, totient, true)
from diofant.abc import a, b, c, w, x, y, z
from diofant.concrete.expr_with_intlimits import ExprWithIntLimits
from diofant.concrete.expr_with_limits import AddWithLimits, ExprWithLimits
Expand Down Expand Up @@ -1849,10 +1849,6 @@ def test_diofant__matrices__expressions__factorizations__Factorization():
pass


def test_diofant__core__numbers__AlgebraicNumber():
assert _test_args(AlgebraicNumber(sqrt(2), [1, 2, 3]))


def test_diofant__polys__polytools__GroebnerBasis():
assert _test_args(GroebnerBasis([x, y, z], x, y, z))

Expand Down
22 changes: 4 additions & 18 deletions diofant/core/tests/test_numbers.py
Expand Up @@ -8,11 +8,10 @@
from mpmath import mpf
from mpmath.libmp.libmpf import _normalize, finf, fnan, fninf

from diofant import (AlgebraicNumber, Catalan, E, EulerGamma, Float, Ge,
GoldenRatio, Gt, I, Integer, Le, Lt, Mul, Number, Pow,
Rational, Symbol, cbrt, cos, exp, factorial, false, latex,
log, nan, nextprime, oo, pi, root, simplify, sin, sqrt,
true, zoo)
from diofant import (Catalan, E, EulerGamma, Float, Ge, GoldenRatio, Gt, I,
Integer, Le, Lt, Mul, Number, Pow, Rational, Symbol, cbrt,
cos, exp, factorial, false, latex, log, nan, nextprime,
oo, pi, root, sin, sqrt, true, zoo)
from diofant.core.cache import clear_cache
from diofant.core.numbers import (comp, igcd, igcdex, ilcm, mod_inverse,
mpf_norm, seterr)
Expand Down Expand Up @@ -1495,19 +1494,6 @@ def test_sympyissue_7742():
assert -oo % 1 == nan


def test_simplify_AlgebraicNumber():
A = AlgebraicNumber
e = root(3, 6)*(3 + (135 + 78*sqrt(3))**Rational(2, 3))/cbrt(45 + 26*sqrt(3))
assert simplify(A(e)) == A(12) # wester test_C20
assert simplify(A(12)) == A(12)

e = root(41 + 29*sqrt(2), 5)
assert simplify(A(e)) == A(1 + sqrt(2)) # wester test_C21

e = (3 + 4*I)**Rational(3, 2)
assert simplify(A(e)) == A(2 + 11*I) # issue sympy/sympy#4401


def test_Float_idempotence():
x = Float('1.23', '')
y = Float(x)
Expand Down
3 changes: 2 additions & 1 deletion diofant/domains/algebraicfield.py
Expand Up @@ -10,6 +10,7 @@
from ..polys.densetools import dmp_compose
from ..polys.euclidtools import dup_invert
from ..polys.polyerrors import CoercionFailed, DomainError, NotAlgebraic
from ..printing.defaults import DefaultPrinting
from .characteristiczero import CharacteristicZero
from .domainelement import DomainElement
from .field import Field
Expand Down Expand Up @@ -140,7 +141,7 @@ def is_nonnegative(self, a):
return self.domain.is_nonnegative(a.LC())


class AlgebraicElement(DomainElement, CantSympify):
class AlgebraicElement(DomainElement, CantSympify, DefaultPrinting):
"""Dense Algebraic Number Polynomials over a field. """

def __init__(self, rep):
Expand Down
16 changes: 2 additions & 14 deletions diofant/polys/numberfields.py
Expand Up @@ -5,8 +5,8 @@

import mpmath

from ..core import (Add, AlgebraicNumber, Dummy, E, GoldenRatio, I, Integer,
Mul, Rational, S, pi, prod, sympify)
from ..core import (Add, Dummy, E, GoldenRatio, I, Mul, Rational, S, pi, prod,
sympify)
from ..core.exprtools import Factors
from ..core.function import _mexpand
from ..domains import QQ, ZZ, AlgebraicField
Expand Down Expand Up @@ -513,8 +513,6 @@ def _minpoly_compose(ex, x, dom):
res = _minpoly_cos(ex, x)
elif ex.__class__ is RootOf:
res = _minpoly_rootof(ex, x)
elif ex.__class__ is AlgebraicNumber:
res = minpoly_groebner(ex, x, dom)
else:
raise NotAlgebraic("%s doesn't seem to be an algebraic element" % ex)
return res
Expand Down Expand Up @@ -634,16 +632,6 @@ def bottom_up_scan(ex):
bmp = PurePoly(minpoly_groebner(1/base, x, domain=domain), x)
base, exp = update_mapping(1/base, bmp), -exp
return update_mapping(ex, exp.q, -base**exp.p)
elif ex.is_AlgebraicNumber:
base = update_mapping(ex.root, ex.minpoly)
res = Integer(0)
for exp, coeff in ex.rep.terms():
exp = Integer(exp[0])
if exp:
res += coeff*update_mapping(base**exp, 1, -base**exp)
else:
res += coeff
return res
elif isinstance(ex, RootOf) and ex.poly.domain.is_IntegerRing:
return update_mapping(ex, ex.poly)

Expand Down

0 comments on commit cb6e730

Please sign in to comment.