Skip to content

Commit

Permalink
Merge pull request #7708 from smichr/4077
Browse files Browse the repository at this point in the history 
4077: use signsimp as first step in cancel
  • Loading branch information
@smichr
smichr committed Nov 29, 2021
2 parents 649cd16 + a48b36d commit ed0bfd1
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 43 deletions.
4 changes: 2 additions & 2 deletions sympy/integrals/tests/test_heurisch.py
View file
Expand Up @@ -220,8 +220,8 @@ def test_heurisch_wrapper():
(-log(x - y)/(2*y) + log(x + y)/(2*y), Ne(y, 0)), (1/x, True))
# issue 6926
f = sqrt(x**2/((y - x)*(y + x)))
assert heurisch_wrapper(f, x) == x*sqrt(x**2/(-x**2 + y**2)) \
- y**2*sqrt(x**2/(-x**2 + y**2))/x
assert heurisch_wrapper(f, x) == x*sqrt(-x**2/(x**2 - y**2)) \
- y**2*sqrt(-x**2/(x**2 - y**2))/x

def test_issue_3609():
assert heurisch(1/(x * (1 + log(x)**2)), x) == atan(log(x))
Expand Down
34 changes: 17 additions & 17 deletions sympy/physics/control/lti.py
View file
Expand Up @@ -2248,13 +2248,13 @@ def doit(self, cancel=True, expand=False, **kwargs):
| [ - -----] [- -] | [ - -----]
\ [ 1 s ]{t} [1 1]{t}/ [ 1 s ]{t}
>>> pprint(F_1.doit(), use_unicode=False)
[ -s 1 - s ]
[ ------- ----------- ]
[ 6*s - 1 s*(1 - 6*s) ]
[ ]
[25*s*(s - 1) + 5*(1 - s)*(6*s - 1) (s - 1)*(6*s + 24)]
[---------------------------------- ------------------]
[ (1 - s)*(6*s - 1) s*(6*s - 1) ]{t}
[ -s s - 1 ]
[------- ----------- ]
[6*s - 1 s*(6*s - 1) ]
[ ]
[5*s - 5 (s - 1)*(6*s + 24)]
[------- ------------------]
[6*s - 1 s*(6*s - 1) ]{t}
If the user wants the resultant ``TransferFunctionMatrix`` object without
canceling the common factors then the ``cancel`` kwarg should be passed ``False``.
Expand All @@ -2273,16 +2273,16 @@ def doit(self, cancel=True, expand=False, **kwargs):
the ``expand`` kwarg should be passed as ``True``.
>>> pprint(F_1.doit(expand=True), use_unicode=False)
[ -s 1 - s ]
[ ------- ---------- ]
[ 6*s - 1 2 ]
[ - 6*s + s ]
[ ]
[ 2 2 ]
[- 5*s + 10*s - 5 6*s + 18*s - 24]
[----------------- ----------------]
[ 2 2 ]
[ - 6*s + 7*s - 1 6*s - s ]{t}
[ -s s - 1 ]
[------- -------- ]
[6*s - 1 2 ]
[ 6*s - s ]
[ ]
[ 2 ]
[5*s - 5 6*s + 18*s - 24]
[------- ----------------]
[6*s - 1 2 ]
[ 6*s - s ]{t}
"""
_mat = self.sensitivity * self.sys1.doit()._expr_mat
Expand Down
2 changes: 1 addition & 1 deletion sympy/physics/control/tests/test_lti.py
View file
Expand Up @@ -1077,7 +1077,7 @@ def test_MIMOFeedback_functions():
(TransferFunction(0, 1, s), TransferFunction(-s**2 + s, 1, s))))
assert F_2.doit() == \
TransferFunctionMatrix(((TransferFunction(s*(-2*s**2 + s*(2*s - 1) + 1), s**2 - s + 1, s),
TransferFunction(2*s**3*(1 - s), s**2 - s + 1, s)), (TransferFunction(0, 1, s), TransferFunction(s*(1 - s), 1, s))))
TransferFunction(-2*s**3*(s - 1), s**2 - s + 1, s)), (TransferFunction(0, 1, s), TransferFunction(s*(1 - s), 1, s))))
assert F_2.doit(expand=True) == \
TransferFunctionMatrix(((TransferFunction(-s**2 + s, s**2 - s + 1, s), TransferFunction(-2*s**4 + 2*s**3, s**2 - s + 1, s)),
(TransferFunction(0, 1, s), TransferFunction(-s**2 + s, 1, s))))
Expand Down
5 changes: 4 additions & 1 deletion sympy/polys/polytools.py
View file
Expand Up @@ -6647,7 +6647,7 @@ def nth_power_roots_poly(f, n, *gens, **args):


@public
def cancel(f, *gens, **args):
def cancel(f, *gens, _signsimp=True, **args):
"""
Cancel common factors in a rational function ``f``.
Expand All @@ -6671,11 +6671,14 @@ def cancel(f, *gens, **args):
>>> together(_)
(x + 2)/2
"""
from sympy.simplify.simplify import signsimp
from sympy.functions.elementary.piecewise import Piecewise
from sympy.polys.rings import sring
options.allowed_flags(args, ['polys'])

f = sympify(f)
if _signsimp:
f = signsimp(f)
opt = {}
if 'polys' in args:
opt['polys'] = args['polys']
Expand Down
39 changes: 39 additions & 0 deletions sympy/polys/tests/test_polytools.py
View file
Expand Up @@ -74,6 +74,7 @@
from sympy.matrices.dense import Matrix
from sympy.matrices.expressions.matexpr import MatrixSymbol
from sympy.polys.rootoftools import rootof
from sympy.simplify.simplify import signsimp
from sympy.utilities.iterables import iterable

from sympy.testing.pytest import raises, warns_deprecated_sympy
Expand Down Expand Up @@ -3087,6 +3088,44 @@ def test_cancel():
assert cancel((x**2 - 1)/(x + 1)*p3) == (x - 1)*p4
assert cancel((x**2 - 1)/(x + 1) + p3) == (x - 1) + p4

# issue 4077
q = S('''(2*1*(x - 1/x)/(x*(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x -
1/x)) - 2/x)) - 2*1*((x - 1/x)/((x*(x - 1/x)**2)) - 1/(x*(x -
1/x)))*((-x + 1/x)*((x - 1/x)/((x*(x - 1/x)**2)) - 1/(x*(x -
1/x)))/(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x - 1/x)) -
2/x) + 1)*((x - 1/x)/((x - 1/x)**2) - ((x - 1/x)/((x*(x - 1/x)**2)) -
1/(x*(x - 1/x)))**2/(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x
- 1/x)) - 2/x) - 1/(x - 1/x))*(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) -
1/(x**2*(x - 1/x)) - 2/x)/x - 1/x)*(((-x + 1/x)/((x*(x - 1/x)**2)) +
1/(x*(x - 1/x)))*((-(x - 1/x)/(x*(x - 1/x)) - 1/x)*((x - 1/x)/((x*(x -
1/x)**2)) - 1/(x*(x - 1/x)))/(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) -
1/(x**2*(x - 1/x)) - 2/x) - 1 + (x - 1/x)/(x - 1/x))/((x*((x -
1/x)/((x - 1/x)**2) - ((x - 1/x)/((x*(x - 1/x)**2)) - 1/(x*(x -
1/x)))**2/(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x - 1/x)) -
2/x) - 1/(x - 1/x))*(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x
- 1/x)) - 2/x))) + ((x - 1/x)/((x*(x - 1/x))) + 1/x)/((x*(2*x - (-x +
1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x - 1/x)) - 2/x))) + 1/x)/(2*x +
2*((x - 1/x)/((x*(x - 1/x)**2)) - 1/(x*(x - 1/x)))*((-(x - 1/x)/(x*(x
- 1/x)) - 1/x)*((x - 1/x)/((x*(x - 1/x)**2)) - 1/(x*(x - 1/x)))/(2*x -
(-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x - 1/x)) - 2/x) - 1 + (x -
1/x)/(x - 1/x))/((x*((x - 1/x)/((x - 1/x)**2) - ((x - 1/x)/((x*(x -
1/x)**2)) - 1/(x*(x - 1/x)))**2/(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2)
- 1/(x**2*(x - 1/x)) - 2/x) - 1/(x - 1/x))*(2*x - (-x + 1/x)/(x**2*(x
- 1/x)**2) - 1/(x**2*(x - 1/x)) - 2/x))) - 2*((x - 1/x)/((x*(x -
1/x))) + 1/x)/(x*(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x -
1/x)) - 2/x)) - 2/x) - ((x - 1/x)/((x*(x - 1/x)**2)) - 1/(x*(x -
1/x)))*((-x + 1/x)*((x - 1/x)/((x*(x - 1/x)**2)) - 1/(x*(x -
1/x)))/(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x - 1/x)) -
2/x) + 1)/(x*((x - 1/x)/((x - 1/x)**2) - ((x - 1/x)/((x*(x - 1/x)**2))
- 1/(x*(x - 1/x)))**2/(2*x - (-x + 1/x)/(x**2*(x - 1/x)**2) -
1/(x**2*(x - 1/x)) - 2/x) - 1/(x - 1/x))*(2*x - (-x + 1/x)/(x**2*(x -
1/x)**2) - 1/(x**2*(x - 1/x)) - 2/x)) + (x - 1/x)/((x*(2*x - (-x +
1/x)/(x**2*(x - 1/x)**2) - 1/(x**2*(x - 1/x)) - 2/x))) - 1/x''',
evaluate=False)
assert cancel(q, _signsimp=False) == -1/(2*x)
assert q.subs(x, 2) is S.NaN
assert signsimp(q) is S.NaN

# issue 9363
M = MatrixSymbol('M', 5, 5)
assert cancel(M[0,0] + 7) == M[0,0] + 7
Expand Down
17 changes: 12 additions & 5 deletions sympy/series/gruntz.py
View file
Expand Up @@ -489,14 +489,21 @@ def calculate_series(e, x, logx=None):
This is a place that fails most often, so it is in its own function.
"""
from sympy.polys import cancel

for t in e.lseries(x, logx=logx):
# bottom_up function is required for a specific case - when e is
# -exp(p/(p + 1)) + exp(-p**2/(p + 1) + p). No current simplification
# methods reduce this to 0 while not expanding polynomials.
t = bottom_up(t, lambda w: getattr(w, 'normal', lambda: w)())
t = cancel(t, expand=False).factor()
# -exp(p/(p + 1)) + exp(-p**2/(p + 1) + p)
t = bottom_up(t, lambda w:
getattr(w, 'normal', lambda: w)())
# And the expression
# `(-sin(1/x) + sin((x + exp(x))*exp(-x)/x))*exp(x)`
# from the first test of test_gruntz_eval_special needs to
# be expanded. But other forms need to be have at least
# factor_terms applied. `factor` accomplishes both and is
# faster than using `factor_terms` for the gruntz suite. It
# does not appear that use of `cancel` is necessary.
# t = cancel(t, expand=False)
t = t.factor()

if t.has(exp) and t.has(log):
t = powdenest(t)
Expand Down
4 changes: 2 additions & 2 deletions sympy/series/tests/test_gruntz.py
View file
Expand Up @@ -398,12 +398,12 @@ def test_MrvTestCase_page47_ex3_21():
assert mmrv(expr, x) == {1/h, exp(-x), exp(x), exp(x - h), exp(x/(1 + h))}


def test_I():
def test_gruntz_I():
y = Symbol("y")
assert gruntz(I*x, x, oo) == I*oo
assert gruntz(y*I*x, x, oo) == y*I*oo
assert gruntz(y*3*I*x, x, oo) == y*I*oo
assert gruntz(y*3*sin(I)*x, x, oo).simplify().rewrite(sign) == y*I*oo
assert gruntz(y*3*sin(I)*x, x, oo) == y*I*oo


def test_issue_4814():
Expand Down
4 changes: 2 additions & 2 deletions sympy/series/tests/test_series.py
View file
Expand Up @@ -344,8 +344,8 @@ def test_issue_20697():
def test_issue_21245():
fi = (1 + sqrt(5))/2
assert (1/(1 - x - x**2)).series(x, 1/fi, 1).factor() == \
(-6964*sqrt(5) - 15572 + 2440*sqrt(5)*x + 5456*x\
+ O((x - 2/(1 + sqrt(5)))**2, (x, 2/(1 + sqrt(5)))))/((1 + sqrt(5))**2\
(-4812 - 2152*sqrt(5) + 1686*x + 754*sqrt(5)*x\
+ O((x - 2/(1 + sqrt(5)))**2, (x, 2/(1 + sqrt(5)))))/((1 + sqrt(5))\
*(20 + 9*sqrt(5))**2*(x + sqrt(5)*x - 2))


Expand Down
6 changes: 4 additions & 2 deletions sympy/simplify/simplify.py
View file
Expand Up @@ -402,7 +402,9 @@ def signsimp(expr, evaluate=None):
expr = sympify(expr)
if not isinstance(expr, (Expr, Relational)) or expr.is_Atom:
return expr
e = sub_post(sub_pre(expr))
# get rid of an pre-existing unevaluation regarding sign
e = expr.replace(lambda x: x.is_Mul and -(-x) != x, lambda x: -(-x))
e = sub_post(sub_pre(e))
if not isinstance(e, (Expr, Relational)) or e.is_Atom:
return e
if e.is_Add:
Expand All @@ -412,7 +414,7 @@ def signsimp(expr, evaluate=None):
return Mul(S.NegativeOne, -rv, evaluate=False)
return rv
if evaluate:
e = e.xreplace({m: -(-m) for m in e.atoms(Mul) if -(-m) != m})
e = e.replace(lambda x: x.is_Mul and -(-x) != x, lambda x: -(-x))
return e


Expand Down
22 changes: 11 additions & 11 deletions sympy/utilities/tests/test_wester.py
View file
Expand Up @@ -918,17 +918,17 @@ def test_M6():

def test_M7():
# TODO: Replace solve with solveset, as of now test fails for solveset
sol = solve(x**8 - 8*x**7 + 34*x**6 - 92*x**5 + 175*x**4 - 236*x**3 +
226*x**2 - 140*x + 46, x)
assert [s.simplify() for s in sol] == [
1 - sqrt(-6 - 2*I*sqrt(3 + 4*sqrt(3)))/2,
1 + sqrt(-6 - 2*I*sqrt(3 + 4*sqrt(3)))/2,
1 - sqrt(-6 + 2*I*sqrt(3 + 4*sqrt(3)))/2,
1 + sqrt(-6 + 2*I*sqrt(3 + 4*sqrt (3)))/2,
1 - sqrt(-6 + 2*sqrt(-3 + 4*sqrt(3)))/2,
1 + sqrt(-6 + 2*sqrt(-3 + 4*sqrt(3)))/2,
1 - sqrt(-6 - 2*sqrt(-3 + 4*sqrt(3)))/2,
1 + sqrt(-6 - 2*sqrt(-3 + 4*sqrt(3)))/2]
assert set(solve(x**8 - 8*x**7 + 34*x**6 - 92*x**5 + 175*x**4 - 236*x**3 +
226*x**2 - 140*x + 46, x)) == set([
1 - sqrt(2)*I*sqrt(-sqrt(-3 + 4*sqrt(3)) + 3)/2,
1 - sqrt(2)*sqrt(-3 + I*sqrt(3 + 4*sqrt(3)))/2,
1 - sqrt(2)*I*sqrt(sqrt(-3 + 4*sqrt(3)) + 3)/2,
1 - sqrt(2)*sqrt(-3 - I*sqrt(3 + 4*sqrt(3)))/2,
1 + sqrt(2)*I*sqrt(sqrt(-3 + 4*sqrt(3)) + 3)/2,
1 + sqrt(2)*sqrt(-3 - I*sqrt(3 + 4*sqrt(3)))/2,
1 + sqrt(2)*sqrt(-3 + I*sqrt(3 + 4*sqrt(3)))/2,
1 + sqrt(2)*I*sqrt(-sqrt(-3 + 4*sqrt(3)) + 3)/2,
])


@XFAIL # There are an infinite number of solutions.
Expand Down

0 comments on commit ed0bfd1

Please sign in to comment.