-
Couldn't load subscription status.
- Fork 43
N speedup #285
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
N speedup #285
Conversation
|
This also reorganizes code to put like things together. The upshot here is Before and And after and The last conversion is 10 times faster, the others faster as they require fewer allocations. The main performance improvement being dispatch based on |
|
Btw, following should reduce the memory allocations, but doesn't get anywhere near this PR. diff --git a/src/types.jl b/src/types.jl
index 435d995..b9c3512 100644
--- a/src/types.jl
+++ b/src/types.jl
@@ -106,9 +106,24 @@ function get_class_from_id(id::UInt)
str
end
-"Get SymEngine class of an object (e.g. 1=>:Integer, 1//2 =:Rational, sin(x) => :Sin, ..."
-get_symengine_class(s::Basic) = Symbol(get_class_from_id(get_type(s)))
+function get_symengine_classes()
+ d = Vector{Symbol}()
+ i = 0
+ while true
+ s = get_class_from_id(UInt(i))
+ if s == ""
+ break
+ end
+ push!(d, Symbol(s))
+ i+=1
+ end
+ return d
+end
+symengine_classes = get_symengine_classes()
+
+"Get SymEngine class of an object (e.g. 1=>:Integer, 1//2 =:Rational, sin(x) => :Sin, ..."
+get_symengine_class(s::Basic) = symengine_classes[get_type(s) + 1]
## Construct symbolic objects
## renamed, as `Symbol` conflicts with Base.Symbol |
|
Ohh, interesting. I was thinking of using a dictionary to cache calls based on the value of |
|
Hi @isuruf, can I merge this? |
|
@jverzani can you check that the last commit doesn't make things slowdown? |
|
Hi @isuruf, I like the dispatch style as the code is much clearer, but for these two basic types it seems a bit slower: AfterBeforeI modified the calling order of |
|
[I updated this, I used the wrong branch in testing which didn't have the improvements to Hi @isuruf. Here is a quick little benchmark of different styles to compare the performance of runtime dispatch versus a simple ifelse switch: This gives I was expecting Nd to be similar to Nc, but there is an extra allocation. I think the construction of a |
|
Can you try the following? using SymEngine, BenchmarkTools
using SymEngine: BasicType, is_a_Integer, _N_Integer,
is_a_RealDouble, _N_RealDouble,
evalf, get_symengine_class
# non mutating
struct _BasicType{T} <: Number
x::Basic
end
_BasicType(x::Basic) = _BasicType{Val{get_symengine_class(x)}}(x)
Basic(x::_BasicType) = x.x
## no dispatch
function Na(x::Basic)
if is_a_Integer(x)
return _N_Integer(x)
elseif is_a_RealDouble(x)
return _N_RealDouble(x)
else
return _N_RealDouble(evalf(x, 53, true))
end
end
## BasicType
Nb(x::Basic) = Nb(BasicType(x))
Nb(x::BasicType{Val{:Integer}}) = _N_Integer(Basic(x))
Nb(x::BasicType{Val{:RealDouble}}) = _N_RealDouble(Basic(x))
Nb(x::BasicType{Val{T}}) where {T} = Nb(SymEngine.evalf(Basic(x), 53, true))
const symengine_classes_val = [Val(c) for c in SymEngine.symengine_classes]
## Val
Nc(x::Basic) = Nc(symengine_classes_val[SymEngine.get_type(x) + 1], x)
Nc(::Val{:Integer}, x::Basic) = _N_Integer(x)
Nc(::Val{:RealDouble}, x::Basic) = _N_RealDouble(x)
Nc(::Val{<:Any}, x::Basic) = Nc(evalf(x, 53, true))
## non-mutable BasicType
Nd(x::Basic) = Nd(_BasicType(x))
Nd(x::_BasicType{Val{:Integer}}) = _N_Integer(Basic(x))
Nd(x::_BasicType{Val{:RealDouble}}) = _N_RealDouble(Basic(x))
Nd(x::_BasicType{Val{T}}) where {T} = Nd(evalf(Basic(x), 53, true))
const integer_type_id::UInt64 = findfirst(==(:Integer), SymEngine.symengine_classes) - 1
const real_double_type_id::UInt64 = findfirst(==(:RealDouble), SymEngine.symengine_classes) - 1
function Ne(x::Basic)
t::UInt64 = SymEngine.get_type(x)
if t == 0
return _N_Integer(x)
elseif t == 6
return _N_RealDouble(x)
else
return _N_RealDouble(evalf(x, 53, true))
end
end
for meth in (Na,Nb,Nc,Nd,Ne)
@show nameof(meth)
@btime $meth(a) setup=(a=Basic(rand(1:10)))
@btime $meth(a) setup=(a=Basic(rand()))
@btime $meth(a) setup=(a=Basic(big(rand())))
end |
|
This is great! I now see: The |
|
Thanks so much. With this most recent tweak, I'm seeing this: I don't know why there is the 1 allocation for a Float64 (there were 0 in the test cases) but this is much more performant that the currently tagged version: |
|
Hi @isuruf, I'm done tinkering here, unless you have other ideas. |
This speeds up
Nby avoiding use ofBasicTypeto control dispatch in favor of multipleifelsechecks.