Merge pull request #422 from JuliaSymbolics/s/simplify2

Fixes to simplify_fractions, call it in simplify by default

Author: shashi

src/SymbolicUtils.jl

@@ -54,8 +54,11 @@ include("ordering.jl")
 include("simplify_rules.jl")
 
 # API = simplify + substitute
-export simplify, substitute
-include("api.jl")
+export simplify
+include("simplify.jl")
+
+export substitute
+include("substitute.jl")
 
 # EGraph rewriting
 include("egraph.jl")

src/polyform.jl

@@ -274,6 +274,14 @@ add_divs(x::Div, y) = (x.num + y * x.den) / x.den
 add_divs(x, y::Div) = (x * y.den + y.num) / y.den
 add_divs(x, y) = x + y
 
+function frac_similarterm(x, f, args; kw...)
+    if f in (*, /, \, +, -, ^)
+        f(args...)
+    else
+        similarterm(x, f, args; kw...)
+    end
+end
+
 """
     simplify_fractions(x; polyform=false)
 
@@ -290,7 +298,9 @@ function simplify_fractions(x; polyform=false)
 
     sdiv(a) = a isa Div ? simplify_div(a) : a
 
-    expr = Postwalk(sdiv ∘ quick_cancel)(Postwalk(add_with_div)(x))
+    expr = Postwalk(sdiv ∘ quick_cancel,
+                    similarterm=frac_similarterm)(Postwalk(add_with_div,
+                                                           similarterm=frac_similarterm)(x))
 
     polyform ? expr : unpolyize(expr)
 end

src/simplify.jl

@@ -0,0 +1,51 @@
+
+"""
+```julia
+simplify(x; expand=false,
+            threaded=false,
+            thread_subtree_cutoff=100,
+            nonzero_denominators=true,
+            rewriter=nothing)
+```
+
+Simplify an expression (`x`) by applying `rewriter` until there are no changes.
+`expand=true` applies [`expand`](/api/#expand) in the beginning of each fixpoint iteration.
+
+By default, simplify will assume denominators are not zero and allow cancellation in fractions.
+Pass `simplify_fractions=false` to prevent this.
+"""
+function simplify(x;
+                  expand=false,
+                  polynorm=nothing,
+                  threaded=false,
+                  simplify_fractions=true,
+                  thread_subtree_cutoff=100,
+                  rewriter=nothing)
+    if polynorm !== nothing
+        Base.depwarn("simplify(..; polynorm=$polynorm) is deprecated, use simplify(..; expand=$polynorm) instead",
+                        :simplify)
+    end
+
+
+    f = if rewriter === nothing
+        if threaded
+            threaded_simplifier(thread_subtree_cutoff)
+        elseif expand
+            serial_expand_simplifier
+        else
+            serial_simplifier
+        end
+    else
+        Fixpoint(rewriter)
+    end
+
+    x = PassThrough(f)(x)
+    simplify_fractions && has_operation(x, /) ?
+        SymbolicUtils.simplify_fractions(x) : x
+end
+
+has_operation(x, op) = (istree(x) && (operation(x) == op ||
+                                      any(a->has_operation(a, op),
+                                          unsorted_arguments(x))))
+
+Base.@deprecate simplify(x, ctx; kwargs...)  simplify(x; rewriter=ctx, kwargs...)

src/api.jl renamed to src/substitute.jl

@@ -1,43 +1,3 @@
-##### Numeric simplification
-
-"""
-```julia
-simplify(x; expand=false,
-            threaded=false,
-            thread_subtree_cutoff=100,
-            rewriter=nothing)
-```
-
-Simplify an expression (`x`) by applying `rewriter` until there are no changes.
-`expand=true` applies [`expand`](/api/#expand) in the beginning of each fixpoint iteration.
-"""
-function simplify(x;
-                  expand=false,
-                  polynorm=nothing,
-                  threaded=false,
-                  thread_subtree_cutoff=100,
-                  rewriter=nothing)
-    if polynorm !== nothing
-        Base.depwarn("simplify(..; polynorm=$polynorm) is deprecated, use simplify(..; expand=$polynorm) instead",
-                        :simplify)
-    end
-
-    f = if rewriter === nothing
-        if threaded
-            threaded_simplifier(thread_subtree_cutoff)
-        elseif expand
-            serial_expand_simplifier
-        else
-            serial_simplifier
-        end
-    else
-        Fixpoint(rewriter)
-    end
-
-    PassThrough(f)(x)
-end
-
-Base.@deprecate simplify(x, ctx; kwargs...)  simplify(x; rewriter=ctx, kwargs...)
 
 """
     substitute(expr, dict; fold=true)

test/rulesets.jl

@@ -106,6 +106,11 @@ end
     @eqtest simplify(exp(a) * a * exp(b)) == simplify(a*exp(a+b))
 end
 
+@testset "simplify_fractions" begin
+    @syms x y z
+    @eqtest simplify(2*((y + z)/x) - 2*y/x - z/x*2) == 0
+end
+
 @testset "Depth" begin
     @syms x
     R = Rewriters.Postwalk(Rewriters.Chain([@rule(sin(~x) => cos(~x)),