Skip to content

Commit

Permalink
polys: construct_domain() enable algebraic extensions by default
Browse files Browse the repository at this point in the history
Before:
    In [1]: Poly(x**2 + sqrt(2), x)
    Out[1]: Poly(x**2 + sqrt(2), x, domain='EX')

    In [2]: Poly(x**2 + sqrt(2))
    Out[2]: Poly(x**2 + sqrt(2), x, sqrt(2), domain='ZZ')

After:
    In [1]: Poly(x**2 + sqrt(2), x)
    Out[1]: Poly(x**2 + sqrt(2), x, domain='QQ<sqrt(2)>')

    In [2]: Poly(x**2 + sqrt(2))
    Out[2]: Poly(x**2 + sqrt(2), x, sqrt(2), domain='ZZ')

Closes sympy/sympy#5428
Closes sympy/sympy#542814337
  • Loading branch information
skirpichev committed Dec 11, 2018
1 parent ba2017e commit d6a062e
Show file tree
Hide file tree
Showing 16 changed files with 55 additions and 52 deletions.
2 changes: 1 addition & 1 deletion diofant/concrete/gosper.py
Expand Up @@ -37,7 +37,7 @@ def gosper_normal(f, g, n, polys=True):
>>> gosper_normal(4*n + 5, 2*(4*n + 1)*(2*n + 3), n, polys=False)
(1/4, n + 3/2, n + 1/4)
"""
(p, q), opt = parallel_poly_from_expr((f, g), n, field=True, extension=True)
(p, q), opt = parallel_poly_from_expr((f, g), n, field=True)

a, A = p.LC(), p.monic()
b, B = q.LC(), q.monic()
Expand Down
3 changes: 1 addition & 2 deletions diofant/domains/algebraicfield.py
Expand Up @@ -182,8 +182,7 @@ def is_negative(self, a):
def _compute_ext_root(ext, minpoly):
from ..polys import minimal_polynomial

for r in minpoly.all_roots(radicals=False, # pragma: no branch
extension=True):
for r in minpoly.all_roots(radicals=False): # pragma: no branch
if not minimal_polynomial(ext - r)(0):
return r.as_content_primitive()

Expand Down
2 changes: 1 addition & 1 deletion diofant/domains/tests/test_domains.py
Expand Up @@ -291,7 +291,7 @@ def test_Domain_unify_algebraic():
assert sqrt2.unify(rootof) == rootof.unify(sqrt2) == ans

# here domain created from tuple, not Expr
p = Poly(x**3 - sqrt(2)*x - 1, x, extension=True)
p = Poly(x**3 - sqrt(2)*x - 1, x)
sqrt2 = p.domain
assert sqrt2.unify(rootof) == rootof.unify(sqrt2) == ans

Expand Down
2 changes: 1 addition & 1 deletion diofant/integrals/heurisch.py
Expand Up @@ -503,7 +503,7 @@ def find_non_syms(expr):
except PolynomialError:
return
else:
ground, _ = construct_domain(non_syms, field=True, extension=True)
ground, _ = construct_domain(non_syms, field=True)

coeff_ring = ground.poly_ring(*poly_coeffs)
ring = coeff_ring.poly_ring(*V)
Expand Down
6 changes: 3 additions & 3 deletions diofant/polys/constructor.py
Expand Up @@ -15,12 +15,12 @@ def _construct_simple(coeffs, opt):
"""Handle simple domains, e.g.: ZZ, QQ, RR and algebraic domains. """
result, rationals, reals, algebraics = {}, False, False, False

if opt.extension is True:
if opt.extension is False:
def is_algebraic(coeff):
return coeff.is_number and coeff.is_algebraic
return False
else:
def is_algebraic(coeff):
return all(_.is_Rational for _ in coeff.as_real_imag())
return coeff.is_number and coeff.is_algebraic

for coeff in coeffs:
if coeff.is_Rational:
Expand Down
2 changes: 1 addition & 1 deletion diofant/polys/numberfields.py
Expand Up @@ -691,7 +691,7 @@ def primitive_element(extension, **args):
*H, g = groebner(F + (f,), Y + (x,), domain=domain, polys=True)

for i, (h, y) in enumerate(zip(H, Y)):
H[i] = (y - h).eject(*Y).retract(field=True, extension=True)
H[i] = (y - h).eject(*Y).retract(field=True)
if not (H[i].domain.is_RationalField or H[i].domain.is_AlgebraicField):
break # G is not a triangular set
else:
Expand Down
4 changes: 2 additions & 2 deletions diofant/polys/polyoptions.py
Expand Up @@ -526,7 +526,7 @@ def preprocess(cls, extension):
if extension == 1:
return bool(extension)
elif extension == 0:
raise OptionError("'False' is an invalid argument for 'extension'")
return bool(extension)
else:
if not hasattr(extension, '__iter__'):
extension = {extension}
Expand All @@ -541,7 +541,7 @@ def preprocess(cls, extension):
@classmethod
def postprocess(cls, options):
from .. import domains
if 'extension' in options and options['extension'] is not True:
if 'extension' in options and options['extension'] not in (True, False):
options['domain'] = domains.QQ.algebraic_field(
*options['extension'])

Expand Down
4 changes: 2 additions & 2 deletions diofant/polys/polyroots.py
Expand Up @@ -658,14 +658,14 @@ def _integer_basis(poly):
return div


def preprocess_roots(poly, extension=None):
def preprocess_roots(poly):
"""Try to get rid of symbolic coefficients from ``poly``. """
coeff = S.One

_, poly = poly.clear_denoms(convert=True)

poly = poly.primitive()[1]
poly = poly.retract(extension=extension)
poly = poly.retract()

# TODO: This is fragile. Figure out how to make this independent of construct_domain().
if poly.domain.is_Poly and all(c.is_term for c in poly.rep.coeffs()):
Expand Down
10 changes: 5 additions & 5 deletions diofant/polys/polytools.py
Expand Up @@ -608,7 +608,7 @@ def to_exact(self):
result = self.rep.to_exact()
return self.per(result)

def retract(self, field=None, extension=None):
def retract(self, field=None):
"""
Recalculate the ground domain of a polynomial.
Expand Down Expand Up @@ -2380,7 +2380,7 @@ def root(self, index, radicals=True):
from .rootoftools import RootOf
return RootOf(self, index, radicals=radicals)

def real_roots(self, multiple=True, radicals=True, extension=None):
def real_roots(self, multiple=True, radicals=True):
"""
Return a list of real roots with multiplicities.
Expand All @@ -2393,14 +2393,14 @@ def real_roots(self, multiple=True, radicals=True, extension=None):
[RootOf(x**3 + x + 1, 0)]
"""
from .rootoftools import RootOf
reals = RootOf.real_roots(self, radicals=radicals, extension=extension)
reals = RootOf.real_roots(self, radicals=radicals)

if multiple:
return reals
else:
return group(reals, multiple=False)

def all_roots(self, multiple=True, radicals=True, extension=None):
def all_roots(self, multiple=True, radicals=True):
"""
Return a list of real and complex roots with multiplicities.
Expand All @@ -2414,7 +2414,7 @@ def all_roots(self, multiple=True, radicals=True, extension=None):
RootOf(x**3 + x + 1, 2)]
"""
from .rootoftools import RootOf
roots = RootOf.all_roots(self, radicals=radicals, extension=extension)
roots = RootOf.all_roots(self, radicals=radicals)

if multiple:
return roots
Expand Down
29 changes: 12 additions & 17 deletions diofant/polys/rootoftools.py
Expand Up @@ -51,21 +51,17 @@ class RootOf(Expr):
Expand polynomial, enabled default.
evaluate : bool or None, optional
Control automatic evaluation.
extension : bool or None, optional
If enabled, reduce input polynomial to have integer
coefficients.
Examples
========
>>> expand_func(RootOf(x**3 + I*x + 2, 0, extension=True))
>>> expand_func(RootOf(x**3 + I*x + 2, 0))
RootOf(x**6 + 4*x**3 + x**2 + 4, 1)
"""

is_commutative = True

def __new__(cls, f, x, index=None, radicals=True, expand=True,
evaluate=None, extension=None):
def __new__(cls, f, x, index=None, radicals=True, expand=True, evaluate=None):
"""Construct a new ``RootOf`` object for ``k``-th root of ``f``. """
x = sympify(x)

Expand All @@ -79,8 +75,7 @@ def __new__(cls, f, x, index=None, radicals=True, expand=True,
else:
raise ValueError("expected an integer root index, got %s" % index)

poly = PurePoly(f, x, greedy=None if extension else False,
expand=expand, extension=extension)
poly = PurePoly(f, x, greedy=False, expand=expand)

if not poly.is_univariate:
raise PolynomialError("only univariate polynomials are allowed")
Expand Down Expand Up @@ -119,7 +114,7 @@ def __new__(cls, f, x, index=None, radicals=True, expand=True,
if roots is not None:
return roots[index]

coeff, poly = preprocess_roots(poly, extension=extension)
coeff, poly = preprocess_roots(poly)

if poly.domain.is_IntegerRing or poly.domain.is_AlgebraicField:
root = cls._indexed_root(poly, index)
Expand Down Expand Up @@ -234,14 +229,14 @@ def is_number(self):
return not self.free_symbols

@classmethod
def real_roots(cls, poly, radicals=True, extension=None):
def real_roots(cls, poly, radicals=True):
"""Get real roots of a polynomial. """
return cls._get_roots("_real_roots", poly, radicals, extension=extension)
return cls._get_roots("_real_roots", poly, radicals)

@classmethod
def all_roots(cls, poly, radicals=True, extension=None):
def all_roots(cls, poly, radicals=True):
"""Get real and complex roots of a polynomial. """
return cls._get_roots("_all_roots", poly, radicals, extension=extension)
return cls._get_roots("_all_roots", poly, radicals)

@classmethod
def _get_reals_sqf(cls, factor):
Expand Down Expand Up @@ -480,14 +475,14 @@ def _roots_trivial(cls, poly, radicals):
return [r*_ for _ in cls._roots_trivial(poly, radicals)]

@classmethod
def _preprocess_roots(cls, poly, extension):
def _preprocess_roots(cls, poly):
"""Take heroic measures to make ``poly`` compatible with ``RootOf``. """
dom = poly.domain

if not dom.is_Exact:
poly = poly.to_exact()

coeff, poly = preprocess_roots(poly, extension=extension)
coeff, poly = preprocess_roots(poly)
dom = poly.domain

if not dom.is_IntegerRing and poly.LC().is_nonzero is False:
Expand All @@ -507,10 +502,10 @@ def _postprocess_root(cls, root, radicals):
return cls._new(poly, index)

@classmethod
def _get_roots(cls, method, poly, radicals, extension):
def _get_roots(cls, method, poly, radicals):
"""Return postprocessed roots of specified kind. """

coeff, poly = cls._preprocess_roots(poly, extension=extension)
coeff, poly = cls._preprocess_roots(poly)
roots = []

for root in getattr(cls, method)(poly):
Expand Down
20 changes: 13 additions & 7 deletions diofant/polys/tests/test_constructor.py
@@ -1,6 +1,7 @@
"""Tests for tools for constructing domains for expressions. """

from diofant import E, Float, GoldenRatio, Integer, Rational, sin, sqrt
from diofant import (E, Float, GoldenRatio, I, Integer, Poly, Rational, sin,
sqrt)
from diofant.abc import x, y
from diofant.domains import EX, QQ, RR, ZZ
from diofant.polys.constructor import construct_domain
Expand All @@ -19,23 +20,23 @@ def test_construct_domain():
assert construct_domain([Rational(1, 2), Integer(2)]) == (QQ, [QQ(1, 2), QQ(2)])
assert construct_domain([3.14, 1, Rational(1, 2)]) == (RR, [RR(3.14), RR(1.0), RR(0.5)])

assert construct_domain([3.14, sqrt(2)], extension=None) == (EX, [EX(3.14), EX(sqrt(2))])
assert construct_domain([3.14, sqrt(2)], extension=True) == (EX, [EX(3.14), EX(sqrt(2))])
assert construct_domain([sqrt(2), 3.14], extension=True) == (EX, [EX(sqrt(2)), EX(3.14)])
assert construct_domain([3.14, sqrt(2)], extension=False) == (EX, [EX(3.14), EX(sqrt(2))])
assert construct_domain([3.14, sqrt(2)]) == (EX, [EX(3.14), EX(sqrt(2))])
assert construct_domain([sqrt(2), 3.14]) == (EX, [EX(sqrt(2)), EX(3.14)])

assert construct_domain([1, sqrt(2)], extension=None) == (EX, [EX(1), EX(sqrt(2))])
assert construct_domain([1, sqrt(2)], extension=False) == (EX, [EX(1), EX(sqrt(2))])

assert construct_domain([x, sqrt(x)]) == (EX, [EX(x), EX(sqrt(x))])
assert construct_domain([x, sqrt(x), sqrt(y)]) == (EX, [EX(x), EX(sqrt(x)), EX(sqrt(y))])

alg = QQ.algebraic_field(sqrt(2))

assert (construct_domain([7, Rational(1, 2), sqrt(2)], extension=True) ==
assert (construct_domain([7, Rational(1, 2), sqrt(2)]) ==
(alg, [alg(7), alg(Rational(1, 2)), alg(sqrt(2))]))

alg = QQ.algebraic_field(sqrt(2) + sqrt(3))

assert (construct_domain([7, sqrt(2), sqrt(3)], extension=True) ==
assert (construct_domain([7, sqrt(2), sqrt(3)]) ==
(alg, [alg(7), alg(sqrt(2)), alg(sqrt(3))]))

dom = ZZ.poly_ring(x)
Expand Down Expand Up @@ -111,3 +112,8 @@ def test_sympyissue_11538():
assert construct_domain(E)[0] == ZZ.poly_ring(E)
assert (construct_domain(x**2 + 2*x + E) == (ZZ.poly_ring(x, E), ZZ.poly_ring(x, E)(x**2 + 2*x + E)))
assert (construct_domain(x + y + GoldenRatio) == (EX, EX(x + y + GoldenRatio)))


def test_sympyissue_5428_14337():
assert Poly(x**2 + I, x).domain == QQ.algebraic_field(I)
assert Poly(x**2 + sqrt(2), x).domain == QQ.algebraic_field(sqrt(2))
4 changes: 1 addition & 3 deletions diofant/polys/tests/test_polyoptions.py
Expand Up @@ -297,6 +297,7 @@ def test_Gaussian_postprocess():
def test_Extension_preprocess():
assert Extension.preprocess(True) is True
assert Extension.preprocess(1) is True
assert Extension.preprocess(False) is False

assert Extension.preprocess([]) is None

Expand All @@ -305,9 +306,6 @@ def test_Extension_preprocess():

assert Extension.preprocess([sqrt(2), I]) == {sqrt(2), I}

pytest.raises(OptionError, lambda: Extension.preprocess(False))
pytest.raises(OptionError, lambda: Extension.preprocess(0))


def test_Extension_postprocess():
opt = {'extension': {sqrt(2)}}
Expand Down
4 changes: 3 additions & 1 deletion diofant/polys/tests/test_polytools.py
Expand Up @@ -3290,7 +3290,9 @@ def test_poly():
assert poly(x + y, y, x) == Poly(x + y, y, x)

# issue sympy/sympy#12400
assert poly(1/(1 + sqrt(2)), x) == Poly(1/(1 + sqrt(2)), x, domain=EX)
assert (poly(1/(1 + sqrt(2)), x) ==
Poly(1/(1 + sqrt(2)), x,
domain=QQ.algebraic_field(1/(1 + sqrt(2)))))


def test_keep_coeff():
Expand Down
10 changes: 5 additions & 5 deletions diofant/polys/tests/test_rootoftools.py
Expand Up @@ -550,25 +550,25 @@ def test_rewrite():
def test_RootOf_expand_func():
r0 = RootOf(x**3 + x + 1, 0)
assert expand_func(r0) == r0
r0 = RootOf(x**3 + I*x + 2, 0, extension=True)
r0 = RootOf(x**3 + I*x + 2, 0)
assert expand_func(r0) == RootOf(x**6 + 4*x**3 + x**2 + 4, 1)
r1 = RootOf(x**3 + I*x + 2, 1, extension=True)
r1 = RootOf(x**3 + I*x + 2, 1)
assert expand_func(r1) == RootOf(x**6 + 4*x**3 + x**2 + 4, 3)

e = RootOf(x**4 + sqrt(2)*x**3 - I*x + 1, 0, extension=True)
e = RootOf(x**4 + sqrt(2)*x**3 - I*x + 1, 0)
assert expand_func(e) == RootOf(x**16 - 4*x**14 + 8*x**12 - 6*x**10 +
10*x**8 + 5*x**4 + 2*x**2 + 1, 1)


@pytest.mark.slow
def test_RootOf_algebraic():
e = RootOf(sqrt(2)*x**4 + sqrt(2)*x**3 - I*x + sqrt(2), x, 0, extension=True)
e = RootOf(sqrt(2)*x**4 + sqrt(2)*x**3 - I*x + sqrt(2), x, 0)
assert e.interval.as_tuple() == ((Rational(-201, 100), 0),
(Rational(-201, 200), Rational(201, 200)))
assert e.evalf(7) == Float('-1.22731258', dps=7) + I*Float('0.6094138324', dps=7)

t = RootOf(x**5 + 4*x + 2, 0)
e = RootOf(x**4 + t*x + 1, 0, extension=True)
e = RootOf(x**4 + t*x + 1, 0)
assert e.interval.as_tuple() == ((Rational(-201, 200), Rational(-201, 200)),
(Rational(-201, 400), Rational(-201, 400)))
assert e.evalf(7) == Float('-0.7123350278', dps=7) - I*Float('0.8248345032', dps=7)
Expand Down
2 changes: 1 addition & 1 deletion diofant/solvers/solvers.py
Expand Up @@ -890,7 +890,7 @@ def _solve_system(exprs, symbols, **flags):
g = d - i
g = g.as_numer_denom()[0]

poly = g.as_poly(*symbols, extension=True)
poly = g.as_poly(*symbols)

if poly is not None:
polys.append(poly)
Expand Down
3 changes: 3 additions & 0 deletions docs/release/notes-0.10.rst
Expand Up @@ -16,6 +16,7 @@ Major changes

* Stable enumeration of polynomial roots in :class:`~diofant.polys.rootoftools.RootOf`, see :pull:`633` and :pull:`658`.
* Support root isolation for polynomials with algebraic coefficients, see :pull:`673` and :pull:`630`.
* Polynomials with algebraic coefficients will use algebraic number domains per default, see :pull:`478`.

Compatibility breaks
====================
Expand Down Expand Up @@ -150,3 +151,5 @@ These Sympy issues also were addressed:
* :sympyissue:`15561` SymPy's Number.__divmod__ doesn't agree with the builtin divmod
* :sympyissue:`15574` dsolve fails for a system of independent equations
* :sympyissue:`12695` [matrices] remove dead files densearith.py densetools.py and densesolve.py
* :sympyissue:`5428` Should Poly use an algebraic domain by default?
* :sympyissue:`14337` Poly constructor uses domain EX when it's not necessary

0 comments on commit d6a062e

Please sign in to comment.