MasonProtter / Bumper.jl

Bring Your Own Stack
MIT License
152 stars 6 forks source link

MethodErrors caused by StrideArrays.jl overrides #34

Closed MilesCranmer closed 2 weeks ago

MilesCranmer commented 6 months ago

MWE:

In a fresh REPL:

julia> using Bumper: @no_escape, @alloc

julia> using Random: randn!

julia> T = ComplexF32
ComplexF32 (alias for Complex{Float32})

julia> @no_escape begin
           ar = @alloc(T, 100)
           randn!(ar)
           @. ar = cos(ar)
           sum(ar)
       end
109.13606f0 + 4.8591895f0im

However, if I import StrideArrays, I get an error:

julia> using Bumper: @no_escape, @alloc

julia> using StrideArrays

julia> using Random: randn!

julia> T = ComplexF32
ComplexF32 (alias for Complex{Float32})

julia> @no_escape begin
           ar = @alloc(T, 100)
           randn!(ar)
           @. ar = cos(ar)
           sum(ar)
       end
ERROR: MethodError: no method matching vmaterialize!(::PtrArray{…}, ::Base.Broadcast.Broadcasted{…}, ::Val{…}, ::Val{…}, ::Val{…})

Closest candidates are:
  vmaterialize!(::Any, ::Any, ::Val{Mod}, ::Val{UNROLL}) where {Mod, UNROLL}
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:753
  vmaterialize!(::Union{LinearAlgebra.Adjoint{T, A}, LinearAlgebra.Transpose{T, A}}, ::BC, ::Val{Mod}, ::Val{UNROLL}, ::Val{dontbc}) where {T<:Union{Bool, Float16, Float32, Float64, Int16, Int32, Int64, Int8, UInt16, UInt32, UInt64, UInt8, SIMDTypes.Bit}, N, A<:AbstractArray{T, N}, BC<:Union{Base.Broadcast.Broadcasted, LoopVectorization.Product}, Mod, UNROLL, dontbc}
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:682
  vmaterialize!(::AbstractArray{T, N}, ::BC, ::Val{Mod}, ::Val{UNROLL}, ::Val{dontbc}) where {T<:Union{Bool, Float16, Float32, Float64, Int16, Int32, Int64, Int8, UInt16, UInt32, UInt64, UInt8, SIMDTypes.Bit}, N, BC<:Union{Base.Broadcast.Broadcasted, LoopVectorization.Product}, Mod, UNROLL, dontbc}
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:673
  ...

Stacktrace:
 [1] vmaterialize!
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:759 [inlined]
 [2] _materialize!
   @ StrideArrays ~/.julia/packages/StrideArrays/PeLtr/src/broadcast.jl:181 [inlined]
 [3] materialize!(dest::PtrArray{…}, bc::Base.Broadcast.Broadcasted{…})
   @ StrideArrays ~/.julia/packages/StrideArrays/PeLtr/src/broadcast.jl:188
 [4] macro expansion
   @ REPL[5]:4 [inlined]
 [5] macro expansion
   @ ~/.julia/packages/Bumper/eoK0g/src/internals.jl:74 [inlined]
 [6] top-level scope
   @ REPL[5]:1
Some type information was truncated. Use `show(err)` to see complete types.

I think maybe a fallback methods should be used if it doesn't exist?

MasonProtter commented 6 months ago

Hm, yeah that seems to be a problem in StrideArrays.jl's broadcast system, it's just manifesting here because Bumper uses types from StrideArrays.

julia> using StrideArrays

julia> let a = StrideArray{ComplexF32}(undef, 10)
           a .= 1
           a .= cos.(a)
       end
ERROR: MethodError: no method matching vmaterialize!(::StrideArray{…}, ::Base.Broadcast.Broadcasted{…}, ::Val{…}, ::Val{…}, ::Val{…})

Closest candidates are:
  vmaterialize!(::Any, ::Any, ::Val{Mod}, ::Val{UNROLL}) where {Mod, UNROLL}
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:753
  vmaterialize!(::Union{Adjoint{T, A}, Transpose{T, A}}, ::BC, ::Val{Mod}, ::Val{UNROLL}, ::Val{dontbc}) where {T<:Union{Bool, Float16, Float32, Float64, Int16, Int32, Int64, Int8, UInt16, UInt32, UInt64, UInt8, SIMDTypes.Bit}, N, A<:AbstractArray{T, N}, BC<:Union{Base.Broadcast.Broadcasted, LoopVectorization.Product}, Mod, UNROLL, dontbc}
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:682
  vmaterialize!(::AbstractArray{T, N}, ::BC, ::Val{Mod}, ::Val{UNROLL}, ::Val{dontbc}) where {T<:Union{Bool, Float16, Float32, Float64, Int16, Int32, Int64, Int8, UInt16, UInt32, UInt64, UInt8, SIMDTypes.Bit}, N, BC<:Union{Base.Broadcast.Broadcasted, LoopVectorization.Product}, Mod, UNROLL, dontbc}
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:673
  ...

Stacktrace:
 [1] vmaterialize!
   @ LoopVectorization ~/.julia/packages/LoopVectorization/7gWfp/src/broadcast.jl:759 [inlined]
 [2] _materialize!
   @ StrideArrays ~/.julia/packages/StrideArrays/PeLtr/src/broadcast.jl:181 [inlined]
 [3] materialize!(dest::StrideArray{…}, bc::Base.Broadcast.Broadcasted{…})
   @ StrideArrays ~/.julia/packages/StrideArrays/PeLtr/src/broadcast.jl:188
 [4] top-level scope
   @ REPL[9]:3

Could you try opening an issue in StrideArrays.jl?

MilesCranmer commented 6 months ago

Thanks, posted there: https://github.com/JuliaSIMD/StrideArrays.jl/issues/85

MilesCranmer commented 6 months ago

It could also be Complex is maybe not supported by StrideArrays? I know that they are not by LoopVectorization at least. For such types maybe @alloc could fall back to regular allocations?

Signature:

T<:Union{Bool, Float16, Float32, Float64, Int16, Int32, Int64, Int8, UInt16, UInt32, UInt64, UInt8, SIMDTypes.Bit}
MasonProtter commented 6 months ago

So what's going on here is that Bumper.jl uses StrideArraysCore.jl because it defines nice pointer-backed performant array types. If one loads StrideArrays.jl, then StrideArraysCore.jl will override various things like broadcasting and matrix multiplication on those types to use LoopVectorization.jl instead of the normal machinery.

That normally works fine, but there are cases where those overrides probably shouldn't be defined, such as when one encounters a Complex, but this isn't really something that Bumper.jl itself has control over, it's really just up to StrideArrays.jl

However, due to issues like this, and the fact that StrideArrays isn't being actively worked on very often, I am considering making a new pointer backed array type to use instead of those from StrideArraysCore.

MilesCranmer commented 4 months ago

Given that LoopVectorization.jl is being deprecated for 1.11, I was wondering what the status of this? I would be very eager to make Bumper.jl a hard dependency for some of my packages, as the perf has been extremely good, but I only hesitate because of the interaction with the soon-to-be-deprecated StrideArrays/LoopVectorization ecosystem. Maybe the PtrArray could simply be copied over so StrideArrays doesn't overload it?

MilesCranmer commented 3 months ago

Friendly ping @MasonProtter

MasonProtter commented 3 months ago

Hey Miles, sorry I want to evaluate options soon. I'm considering switching to UnsafeArrays.jl or something like that, or forking StrideArraysCore or something but haven't had time or bandwidth to deal with it lately. I'm hoping I can get to this soon-ish though

MilesCranmer commented 2 months ago

Thanks!

Btw, I think @alloc should take an explicit array_type argument to avoid this sort of “spooky action at a distance” Then the user would be in charge of specifying StrideArray if they want it, rather than this being something that gets injected behind-the-scenes depending on a hidden global state of the Julia runtime.

MasonProtter commented 2 months ago

Currently planning on just dropping StrideArrays and switching to https://github.com/LilithHafner/PtrArrays.jl/ instead. Should give us everything we need.

MilesCranmer commented 2 months ago

Awesome, looking forward to it!!

MilesCranmer commented 1 month ago

Friendly ping on this