Type instability from keyword argument
Closed this issue · 2 comments
[Suggestion (3) in series started in #14]
I noticed here:
Line 22 in 72fec3c
This results in type instability which can have significant performance effects. In the example below, the performance is reduced by a factor of 100 because of this.
(when there is no keyword present in the call, the compiler can deduce the return type, but if ether b=true
or b=false
is present, it can't find the exact type; it knows only that it is a Union{Int64, Tuple{Int64,Int64}}
)
julia> function f(a; b = false)
if b
return a^2, a^3
else
return a^2
end
end
f (generic function with 1 method)
julia> using BenchmarkTools
julia> @benchmark f(2, b=false)
BenchmarkTools.Trial:
memory estimate: 0 bytes
allocs estimate: 0
--------------
minimum time: 2.095 ns (0.00% GC)
median time: 2.264 ns (0.00% GC)
mean time: 2.313 ns (0.00% GC)
maximum time: 16.213 ns (0.00% GC)
--------------
samples: 10000
evals/sample: 1000
julia> @benchmark f(2, b=$false)
BenchmarkTools.Trial:
memory estimate: 0 bytes
allocs estimate: 0
--------------
minimum time: 2.170 ns (0.00% GC)
median time: 2.536 ns (0.00% GC)
mean time: 2.501 ns (0.00% GC)
maximum time: 474.652 ns (0.00% GC)
--------------
samples: 10000
evals/sample: 1000
julia> @benchmark f(2)
BenchmarkTools.Trial:
memory estimate: 0 bytes
allocs estimate: 0
--------------
minimum time: 0.029 ns (0.00% GC)
median time: 0.032 ns (0.00% GC)
mean time: 0.034 ns (0.00% GC)
maximum time: 13.271 ns (0.00% GC)
--------------
samples: 10000
evals/sample: 1000
julia> @code_warntype f(2)
Variables
#self#::Core.Compiler.Const(f, false)
a::Int64
Body::Int64
1 ─ %1 = Main.:(#f#3)(false, #self#, a)::Int64
└── return %1
julia> @code_warntype f(2, b=false)
Variables
#unused#::Core.Compiler.Const(getfield(Main, Symbol("#kw##f"))(), false)
@_2::NamedTuple{(:b,),Tuple{Bool}}
@_3::Core.Compiler.Const(f, false)
a::Int64
b::Bool
@_6::Bool
Body::Union{Int64, Tuple{Int64,Int64}}
1 ─ %1 = Base.haskey(@_2, :b)::Core.Compiler.Const(true, false)
│ %1
│ (@_6 = Base.getindex(@_2, :b))
└── goto #3
2 ─ Core.Compiler.Const(:(@_6 = false), false)
3 ┄ (b = @_6)
│ %7 = (:b,)::Core.Compiler.Const((:b,), false)
│ %8 = Core.apply_type(Core.NamedTuple, %7)::Core.Compiler.Const(NamedTuple{(:b,),T} where T<:Tuple, false)
│ %9 = Base.structdiff(@_2, %8)::Core.Compiler.Const(NamedTuple(), false)
│ %10 = Base.pairs(%9)::Core.Compiler.Const(Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}(), false)
│ %11 = Base.isempty(%10)::Core.Compiler.Const(true, false)
│ %11
└── goto #5
4 ─ Core.Compiler.Const(:(Base.kwerr(@_2, @_3, a)), false)
5 ┄ %15 = Main.:(#f#3)(b, @_3, a)::Union{Int64, Tuple{Int64,Int64}}
└── return %15
This instance of type instability is fixed in d6a5112. I think it makes sense for update to only return the updated belief. If a user wants a predicted belief, they can run predict separately.
If a user wants a predicted belief, they can run predict separately.
agreed - I think this is the better way to organize it