LoopVectorization.jl icon indicating copy to clipboard operation
LoopVectorization.jl copied to clipboard

Doesn't seem to handle Bool negation well

Open kcajf opened this issue 4 years ago • 3 comments

Hi, just trying @avx for the first time on a simple loop that includes the negation of a boolean:

julia> xs = rand(10)

julia> function foo(x)
           count = 0
           @avx for i in eachindex(x)
               count += !isnan(x[i])
           end
           count
       end
foo (generic function with 1 method)

julia> foo(xs)
ERROR: MethodError: no method matching !(::UInt8)
Closest candidates are:
  !(::Missing) at missing.jl:100
  !(::Bool) at bool.jl:35
  !(::Function) at operators.jl:880
  ...

Is that expected?

kcajf avatar Apr 20 '20 07:04 kcajf

Thanks for the issue. I updated VectorizationBase, SIMDPirates, and LoopVectorization so that now, on the master branches of each:

julia> using LoopVectorization, BenchmarkTools

julia> x = rand(83);

julia> x[rand(1:83,13)] .= NaN;

julia> function foo(x)
           count = 0
           @avx for i in eachindex(x)
               count += !isnan(x[i])
           end
           count
       end
foo (generic function with 1 method)

julia> function foonoavx(x)
           count = 0
           @inbounds @simd for i in eachindex(x)
               count += !isnan(x[i])
           end
           count
       end
foonoavx (generic function with 1 method)

julia> @btime foo($x)
  8.763 ns (0 allocations: 0 bytes)
71

julia> @btime foonoavx($x)
  7.698 ns (0 allocations: 0 bytes)
71

It should be getting the correct answer, but it is slower than @simd.

I'll leave this issue open until I think of a faster way to implement this.

chriselrod avatar Apr 20 '20 21:04 chriselrod

There might be a related problem with ismissing:

julia> function sum_skipmissing_avx(x)
           s = zero(eltype(x))
           @avx for i in eachindex(x)
               if !ismissing(x[i])
                   s += x[i]
               end
           end
           return s
       end
ERROR: LoadError: "Don't know how to handle expression:\nif !(ismissing(x[i]))\n    #= REPL[16]:5 =#\n    s = vadd(s, x[i])\nend"
Stacktrace:
 [1] push!(::LoopVectorization.LoopSet, ::Expr, ::Int64, ::Int64) at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/graphs.jl:563
 [2] add_block!(::LoopVectorization.LoopSet, ::Expr, ::Int64, ::Int64) at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/graphs.jl:336
 [3] add_loop!(::LoopVectorization.LoopSet, ::Expr, ::Int64) at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/graphs.jl:439
 [4] copyto! at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/constructors.jl:6 [inlined]
 [5] LoopVectorization.LoopSet(::Expr, ::Symbol) at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/constructors.jl:52
 [6] LoopVectorization.LoopSet(::Expr, ::Module) at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/constructors.jl:56
 [7] @avx(::LineNumberNode, ::Module, ::Any) at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/constructors.jl:97
in expression starting at REPL[16]:3

The following is mathematically equivalent with above, but throws a different error:

function sum_skipmissing_avx2(x)
    s = zero(eltype(x))
    @avx for i in eachindex(x)
        s += ifelse(ismissing(x[i]), zero(eltype(x)), x[i])
    end
    return s
end
x = convert(Vector{Union{Float64, Missing}}, rand(1000));

julia> sum_skipmissing_avx2(x)
ERROR: KeyError: key Union{Missing, Float64} not found
Stacktrace:
 [1] getindex at ./dict.jl:477 [inlined]
 [2] llvmtype(::Type) at /Users/biona001/.julia/packages/VectorizationBase/tm6zw/src/vectorizable.jl:22
 [3] #s18#93 at /Users/biona001/.julia/packages/SIMDPirates/TPHVh/src/memory.jl:150 [inlined]
 [4] #s18#93(::Any, ::Any, ::Any, ::Any, ::Any, ::Any, ::Any, ::Type, ::Any) at ./none:0
 [5] (::Core.GeneratedFunctionStub)(::Any, ::Vararg{Any,N} where N) at ./boot.jl:524
 [6] vload at /Users/biona001/.julia/packages/SIMDPirates/TPHVh/src/memory.jl:300 [inlined]
 [7] vload at /Users/biona001/.julia/packages/VectorizationBase/tm6zw/src/vectorizable.jl:214 [inlined]
 [8] macro expansion at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/reconstruct_loopset.jl:417 [inlined]
 [9] _avx_! at /Users/biona001/.julia/packages/LoopVectorization/zXjmq/src/reconstruct_loopset.jl:417 [inlined]
 [10] macro expansion at ./gcutils.jl:91 [inlined]
 [11] sum_skipmissing_avx2(::Array{Union{Missing, Float64},1}) at ./REPL[19]:3
 [12] top-level scope at REPL[20]:1

biona001 avatar Apr 30 '20 04:04 biona001

I'll take a look at these later, but I'll need to add explicit support for Union{T,Missing}. That's important in Statistics, and it'd be great if vfilter would work on them as well.

The key is that it'd have to be a pair of pointers. On to the data, and one to the missing indicator. I'm not immediately sure how to get the latter, or to what extent it is user-facing API.

chriselrod avatar May 06 '20 07:05 chriselrod