This has been fixed by https://github.com/JuliaLang/julia/pull/13465
On Fri, Sep 11, 2015 at 9:45 PM, Michael Francis <[email protected]> wrote: > thanks > > On Friday, September 11, 2015 at 7:33:33 PM UTC-4, Stefan Karpinski wrote: >> >> Opened an issue to track this change: >> https://github.com/JuliaLang/julia/issues/13081. >> >> On Wed, Sep 2, 2015 at 11:08 AM, Andreas Noack <[email protected]> >> wrote: >> >>> I think you are right that we should simply remove the mean keyword >>> argument from cov and cor. If users want the efficient versions with user >>> provided means then they can use corm and covm. Right now they are not >>> exported, but we could consider doing it, although I'm in doubt if it is >>> really needed. The important thing is to have cov and cor type stable. >>> >>> >>> On Tuesday, September 1, 2015 at 1:29:59 PM UTC-4, Michael Francis wrote: >>>> >>>> Thanks, that is a good pointer. >>>> >>>> In this specific case its unfortunate that there is a keyword arg in >>>> the API at all, having two functions one with a mean supplied and one >>>> without would avoid this issue and remove the branch logic replacing it >>>> with static dispatch. >>>> >>>> On Tuesday, September 1, 2015 at 1:02:17 PM UTC-4, Jarrett Revels wrote: >>>>> >>>>> Actually, just saw this: >>>>> https://github.com/JuliaLang/julia/issues/9818 >>>>> <https://github.com/JuliaLang/julia/issues/9818>. Ignore the messed >>>>> up @code_typed stuff in my previous reply to this thread. >>>>> >>>>> I believe the type-inference concerns are still there, however, even >>>>> if @code_typed doesn't correctly report them, so the fixes I listed should >>>>> still be useful for patching over inferencing problems with keyword >>>>> arguments. >>>>> >>>>> Best, >>>>> Jarrett >>>>> >>>>> On Tuesday, September 1, 2015 at 12:49:02 PM UTC-4, Jarrett Revels >>>>> wrote: >>>>>> >>>>>> Related: https://github.com/JuliaLang/julia/issues/9551 >>>>>> >>>>>> Unfortunately, as you've seen, type-variadic keyword arguments can >>>>>> really mess up type-inferencing. It appears that keyword argument types >>>>>> are >>>>>> pulled from the default arguments rather than those actually passed in at >>>>>> runtime: >>>>>> >>>>>> *julia> f(x; a=1, b=2) = a*x^b* >>>>>> *f (generic function with 1 method)* >>>>>> >>>>>> *julia> f(1)* >>>>>> *1* >>>>>> >>>>>> *julia> f(1, a=(3+im), b=5.15)* >>>>>> *3.0 + 1.0im* >>>>>> >>>>>> *julia> @code_typed f(1, a=(3+im), b=5.15)* >>>>>> *1-element Array{Any,1}:* >>>>>> * :($(Expr(:lambda, Any[:x], >>>>>> Any[Any[Any[:x,Int64,0]],Any[],Any[Int64],Any[]], :(begin $(Expr(:line, >>>>>> 1, >>>>>> :none, symbol("")))* >>>>>> * GenSym(0) = (Base.power_by_squaring)(x::Int64,2)::Int64* >>>>>> * return (Base.box)(Int64,(Base.mul_int)(1,GenSym(0)))::Int64* >>>>>> * end::Int64))))* >>>>>> >>>>>> Obviously, that specific call to f does NOT return an Int64. >>>>>> >>>>>> I know of only two reasonable ways to handle it at the moment: >>>>>> >>>>>> 1. If you're the method author: Restrict every keyword argument to a >>>>>> declared, concrete type, which ensures that the argument isn't >>>>>> type-variadic. Yichao basically gave an example of this. >>>>>> 2. If you're the method caller: Manually assert the return type. You >>>>>> can do this pretty easily in most cases using a wrapper function. >>>>>> Using `f` from above as an example: >>>>>> >>>>>> *julia> g{X,A,B}(x::X, a::A, b::B) = f(x, a=a, b=b)::promote_type(X, >>>>>> A, B)* >>>>>> *g (generic function with 2 methods)* >>>>>> >>>>>> *julia> @code_typed g(1,2,3)* >>>>>> *1-element Array{Any,1}:* >>>>>> * :($(Expr(:lambda, Any[:x,:a,:b], >>>>>> Any[Any[Any[:x,Int64,0],Any[:a,Int64,0],Any[:b,Int64,0]],Any[],Any[Int64],Any[:X,:A,:B]], >>>>>> :(begin # none, line 1:* >>>>>> * return >>>>>> (top(typeassert))((top(kwcall))((top(getfield))(Main,:call)::F,2,:a,a::Int64,:b,b::Int64,Main.f,(top(ccall))(:jl_alloc_array_1d,(top(apply_type))(Base.Array,Any,1)::Type{Array{Any,1}},(top(svec))(Base.Any,Base.Int)::SimpleVector,Array{Any,1},0,4,0)::Array{Any,1},x::Int64),Int64)::Int64* >>>>>> * end::Int64))))* >>>>>> >>>>>> *julia> @code_typed g(1,2,3.0)* >>>>>> *1-element Array{Any,1}:* >>>>>> * :($(Expr(:lambda, Any[:x,:a,:b], >>>>>> Any[Any[Any[:x,Int64,0],Any[:a,Int64,0],Any[:b,Float64,0]],Any[],Any[Int64],Any[:X,:A,:B]], >>>>>> :(begin # none, line 1:* >>>>>> * return >>>>>> (top(typeassert))((top(kwcall))((top(getfield))(Main,:call)::F,2,:a,a::Int64,:b,b::Float64,Main.f,(top(ccall))(:jl_alloc_array_1d,(top(apply_type))(Base.Array,Any,1)::Type{Array{Any,1}},(top(svec))(Base.Any,Base.Int)::SimpleVector,Array{Any,1},0,4,0)::Array{Any,1},x::Int64),Float64)::Float64* >>>>>> * end::Float64))))* >>>>>> >>>>>> *julia> @code_typed g(1,2,3.0+im)* >>>>>> *1-element Array{Any,1}:* >>>>>> * :($(Expr(:lambda, Any[:x,:a,:b], >>>>>> Any[Any[Any[:x,Int64,0],Any[:a,Int64,0],Any[:b,Complex{Float64},0]],Any[],Any[Int64],Any[:X,:A,:B]], >>>>>> :(begin # none, line 1:* >>>>>> * return >>>>>> (top(typeassert))((top(kwcall))((top(getfield))(Main,:call)::F,2,:a,a::Int64,:b,b::Complex{Float64},Main.f,(top(ccall))(:jl_alloc_array_1d,(top(apply_type))(Base.Array,Any,1)::Type{Array{Any,1}},(top(svec))(Base.Any,Base.Int)::SimpleVector,Array{Any,1},0,4,0)::Array{Any,1},x::Int64),Complex{Float64})::Complex{Float64}* >>>>>> * end::Complex{Float64}))))* >>>>>> >>>>>> Thus, downstream functions can call *f* through *g, *preventing >>>>>> type-instability from "bubbling up" to the calling methods (as it would >>>>>> if >>>>>> they called *f* directly). >>>>>> >>>>>> Best, >>>>>> Jarrett >>>>>> >>>>>> On Tuesday, September 1, 2015 at 8:39:11 AM UTC-4, Michael Francis >>>>>> wrote: >>>>>>> >>>>>>> 2) The underlying functions are only stable if the mean passed to >>>>>>> them is of the correct type, e.g. a number. Essentially this is a type >>>>>>> inference issue, if the compiler was able to optimize the branches >>>>>>> then it >>>>>>> would be likely be ok, it looks from the LLVM code that this is not the >>>>>>> case today. >>>>>>> >>>>>>> FWIW using a type stable version (e.g. directly calling covm) looks >>>>>>> to be about 18% faster for small (100 element) AbstractArray pairs. >>>>>>> >>>>>>> On Monday, August 31, 2015 at 9:06:58 PM UTC-4, Sisyphuss wrote: >>>>>>>> >>>>>>>> IMO: >>>>>>>> 1) This is called keyword argument (not named optional argument). >>>>>>>> 2) The returned value depends only on `corzm`, and `corm`. If these >>>>>>>> two functions are type stable, then `cor` is type stable. >>>>>>>> 3) I'm not sure whether this is the "correct" way to write this >>>>>>>> function. >>>>>>>> >>>>>>>> On Monday, August 31, 2015 at 11:48:37 PM UTC+2, Michael Francis >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> The following is taken from statistics.jl line 428 >>>>>>>>> >>>>>>>>> function cor(x::AbstractVector, y::AbstractVector; mean= >>>>>>>>> nothing) >>>>>>>>> mean == 0 ? corzm(x, y) : >>>>>>>>> mean == nothing ? corm(x, Base.mean(x), y, Base.mean(y)) : >>>>>>>>> isa(mean, (Number,Number)) ? corm(x, mean[1], y, mean[2]) >>>>>>>>> : >>>>>>>>> error("Invalid value of mean.") >>>>>>>>> end >>>>>>>>> >>>>>>>>> due to the 'mean' initially having a type of 'Nothing' I am unable >>>>>>>>> to inference the return type of the function - the following will >>>>>>>>> return >>>>>>>>> Any for the return type. >>>>>>>>> >>>>>>>>> rt = {} >>>>>>>>> for x in Base._methods(f,types,-1) >>>>>>>>> linfo = x[3].func.code >>>>>>>>> (tree, ty) = Base.typeinf(linfo, x[1], x[2]) >>>>>>>>> push!(rt, ty) >>>>>>>>> end >>>>>>>>> >>>>>>>>> Each of the underlying functions are type stable when called >>>>>>>>> directly. >>>>>>>>> >>>>>>>>> Code lowered doesn't give much of a pointer to what will actually >>>>>>>>> happen here, >>>>>>>>> >>>>>>>>> julia> code_lowered( cor, ( Vector{Float64}, Vector{Float64} ) ) >>>>>>>>> 1-element Array{Any,1}: >>>>>>>>> :($(Expr(:lambda, {:x,:y}, {{},{{:x,:Any,0},{:y,:Any,0}},{}}, :( >>>>>>>>> begin $(Expr(:line, 429, symbol("statistics.jl"), symbol(""))) >>>>>>>>> return __cor#195__(nothing,x,y) >>>>>>>>> end)))) >>>>>>>>> >>>>>>>>> >>>>>>>>> If I re-write with a regular optional arg for the mean >>>>>>>>> >>>>>>>>> code_lowered( cordf, ( Vector{Float64}, Vector{Float64}, Nothing ) >>>>>>>>> ) >>>>>>>>> 1-element Array{Any,1}: >>>>>>>>> :($(Expr(:lambda, {:x,:y,:mean}, {{},{{:x,:Any,0},{:y,:Any,0},{: >>>>>>>>> mean,:Any,0}},{}}, :(begin # none, line 2: >>>>>>>>> unless mean == 0 goto 0 >>>>>>>>> return corzm(x,y) >>>>>>>>> 0: >>>>>>>>> unless mean == nothing goto 1 >>>>>>>>> return corm(x,((top(getfield))(Base,:mean))(x),y,((top( >>>>>>>>> getfield))(Base,:mean))(y)) >>>>>>>>> 1: >>>>>>>>> unless isa(mean,(top(tuple))(Number,Number)) goto 2 >>>>>>>>> return corm(x,getindex(mean,1),y,getindex(mean,2)) >>>>>>>>> 2: >>>>>>>>> return error("Invalid value of mean.") >>>>>>>>> end)))) >>>>>>>>> >>>>>>>>> The LLVM code does not look very clean, If I have a real type for >>>>>>>>> the mean (say Float64 ) it looks better 88 lines vs 140 >>>>>>>>> >>>>>>>>> julia> code_llvm( cor, ( Vector{Float64}, Vector{Float64}, Nothing >>>>>>>>> ) ) >>>>>>>>> >>>>>>>>> >>>>>>>>> define %jl_value_t* @julia_cordf_20322(%jl_value_t*, %jl_value_t*, >>>>>>>>> %jl_value_t*) { >>>>>>>>> top: >>>>>>>>> %3 = alloca [7 x %jl_value_t*], align 8 >>>>>>>>> %.sub = getelementptr inbounds [7 x %jl_value_t*]* %3, i64 0, >>>>>>>>> i64 0 >>>>>>>>> %4 = getelementptr [7 x %jl_value_t*]* %3, i64 0, i64 2, !dbg ! >>>>>>>>> 949 >>>>>>>>> store %jl_value_t* inttoptr (i64 10 to %jl_value_t*), % >>>>>>>>> jl_value_t** %.sub, align 8 >>>>>>>>> %5 = getelementptr [7 x %jl_value_t*]* %3, i64 0, i64 1, !dbg ! >>>>>>>>> 949 >>>>>>>>> %6 = load %jl_value_t*** @jl_pgcstack, align 8, ! >>>>>>>>> ... >>>>>>>> >>>>>>>> >>
