Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

similarterm inconsistent with Symbolic arithmetic operations #462

Closed
bowenszhu opened this issue Aug 16, 2022 · 2 comments · Fixed by #480
Closed

similarterm inconsistent with Symbolic arithmetic operations #462

bowenszhu opened this issue Aug 16, 2022 · 2 comments · Fixed by #480

Comments

@bowenszhu
Copy link
Member

bowenszhu commented Aug 16, 2022

similarterm hard-code the resulting Symbolic subtype according to the input operation f. For example, when f is *, similarterm calls Mul no matter what the input args are.

function TermInterface.similarterm(t::Type{<:BasicSymbolic{<:Number}}, f, args, symtype; metadata=nothing, exprhead=:call)
if f isa Symbol
return Term{_promote_symtype(eval(f), args)}(eval(f), args; metadata=metadata)
end
T = symtype
if T === nothing
T = _promote_symtype(f, args)
end
if f === (+)
Add(T, makeadd(1, 0, args...)...; metadata=metadata)
elseif f == (*)
Mul(T, makemul(1, args...)...; metadata=metadata)
elseif f == (/)
@assert length(args) == 2
Div{T}(args...; metadata=metadata)
elseif f == (^) && length(args) == 2
Pow{T}(makepow(args...)...; metadata=metadata)
else
Term{T}(f, args, metadata=metadata)
end
end

However, the result's type is not necessarily Mul for the * operations defined in src/types.jl.

SymbolicUtils.jl/src/types.jl

Lines 1121 to 1162 in 97a8a86

function *(a::SN_EC, b::SN_EC)
# Always make sure Div wraps Mul
if isdiv(a) && isdiv(b)
Div(a.num * b.num, a.den * b.den)
elseif isdiv(a)
Div(a.num * b, a.den)
elseif isdiv(b)
Div(a * b.num, b.den)
elseif ismul(a) && ismul(b)
Mul(mul_t(a, b),
a.coeff * b.coeff,
_merge(+, a.dict, b.dict, filter=_iszero))
elseif ismul(a) && ispow(b)
if b.exp isa Number
Mul(mul_t(a, b),
a.coeff, _merge(+, a.dict, Base.ImmutableDict(b.base=>b.exp), filter=_iszero))
else
Mul(mul_t(a, b),
a.coeff, _merge(+, a.dict, Base.ImmutableDict(b=>1), filter=_iszero))
end
elseif ispow(a) && ismul(b)
b * a
else
Mul(mul_t(a,b), makemul(1, a, b)...)
end
end
function *(a::Number, b::SN_EC)
if iszero(a)
a
elseif isone(a)
b
elseif isdiv(b)
Div(a*b.num, b.den)
elseif isone(-a) && isadd(b)
# -1(a+b) -> -a - b
T = promote_symtype(+, typeof(a), symtype(b))
Add(T, b.coeff * a, Dict{Any,Any}(k=>v*a for (k, v) in b.dict))
else
Mul(mul_t(a, b), makemul(a, b)...)
end
end

See the following test case.

julia> using SymbolicUtils

julia> @syms x y;

julia> s = x + y;

julia> m = -1 * s;

julia> typeof(m)
SymbolicUtils.Add{Number, Int64, Dict{SymbolicUtils.Sym{Number, Nothing}, Int64}, Nothing}

julia> m.coeff
0

julia> m.dict
Dict{SymbolicUtils.Sym{Number, Nothing}, Int64} with 2 entries:
  y => -1
  x => -1

The result of the normal * operation is Add(coeff = 0, dict = Dict(x => -1, y => -1).

julia> using SymbolicUtils

julia> @syms x y;

julia> s = x + y;

julia> f = *;

julia> args = (-1, s);

julia> res = similarterm(SymbolicUtils.Mul, f, args, Real)
-(x + y)

julia> typeof(res)
SymbolicUtils.Mul{Real, Int64, Dict{Any, Number}, Nothing}

julia> res.coeff
-1

julia> res.dict
Dict{Any, Number} with 1 entry:
  x + y => 1

The result of similarterm is Mul(coeff = -1, dict = Dict(Add(coeff = 0, dict = Dict(x => 1, y => 1)) => 1).

@shashi
Copy link
Member

shashi commented Aug 23, 2022

Nice issue! I wonder if just calling * in similarterm would be the way to solve this. Do you have any ideas?

@bowenszhu
Copy link
Member Author

I guess so. The set of Symbolic arithmetic operations are well defined. It seems unnecessary to manually define new rules for the resultant types and data structures in similarterm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants