Skip to content
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

MethodErrors caused by StrideArrays.jl overrides #34

Closed
MilesCranmer opened this issue Jan 10, 2024 · 11 comments · Fixed by #42
Closed

MethodErrors caused by StrideArrays.jl overrides #34

MilesCranmer opened this issue Jan 10, 2024 · 11 comments · Fixed by #42

Comments

@MilesCranmer
Copy link

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
Copy link
Owner

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
Copy link
Author

Thanks, posted there: JuliaSIMD/StrideArrays.jl#85

@MilesCranmer
Copy link
Author

MilesCranmer commented Jan 11, 2024

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
Copy link
Owner

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.

@MasonProtter MasonProtter changed the title Errors depending on what packages are imported MethodErrors caused by StrideArrays.jl overrides Jan 11, 2024
@MilesCranmer
Copy link
Author

MilesCranmer commented Mar 31, 2024

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
Copy link
Author

Friendly ping @MasonProtter

@MasonProtter
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@MilesCranmer
Copy link
Author

Awesome, looking forward to it!!

@MilesCranmer
Copy link
Author

Friendly ping on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants