JuliaSIMD / StrideArrays.jl

Library supporting the ArrayInterface.jl strided array interface.
MIT License
54 stars 9 forks source link

update compat bounds for ArrayInterface and Static; remove unused dependency ThreadingUtilities #42

Closed ranocha closed 2 years ago

ranocha commented 2 years ago

I collected the compat bounds updates of #40 and #41 in this PR. I also removed ThreadingUtilities.jl as direct dependency since it never appeared except in Project.toml.

I also disabled ambiguities testing via Auqa.jl since it results in 21 method ambiguities from ForwardDiff.jl and ChainRulesCore.jl.

Closes #37, closes #38, closes #39, closes #40, closes #41, closes #43, closes #44, closes #45

codecov-commenter commented 2 years ago

Codecov Report

Merging #42 (fcb6ad5) into master (9e768b2) will increase coverage by 0.24%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
+ Coverage   60.00%   60.24%   +0.24%     
==========================================
  Files           4        4              
  Lines         330      332       +2     
==========================================
+ Hits          198      200       +2     
  Misses        132      132              
Impacted Files Coverage Δ
src/miscellaneous.jl 66.15% <100.00%> (+1.07%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2fd96d7...fcb6ad5. Read the comment docs.

chriselrod commented 2 years ago
  StackOverflowError:
  Stacktrace:
       [1] PtrArray
         @ ~/.julia/packages/StrideArraysCore/TzFER/src/ptr_array.jl:7 [inlined]
       [2] PtrArray
         @ ~/.julia/packages/StrideArraysCore/TzFER/src/ptr_array.jl:11 [inlined]
       [3] PtrArray
         @ ~/.julia/packages/StrideArraysCore/TzFER/src/ptr_array.jl:24 [inlined]
       [4] maybe_ptr_array
         @ ~/.julia/packages/StrideArraysCore/TzFER/src/stridearray.jl:150 [inlined]
       [5] maybe_ptr_array
         @ ~/.julia/packages/StrideArraysCore/TzFER/src/stridearray.jl:149 [inlined]
       [6] mapreduce(f::typeof(norm), op::typeof(max), A::PtrArray{Tuple{Int64, Int64}, (true, false), Float64, 2, 1, 0, (1, 2), Tuple{StaticInt{8}, StaticInt{640}}, Tuple{StaticInt{1}, StaticInt{1}}})
         @ StrideArrays ~/work/StrideArrays.jl/StrideArrays.jl/src/miscellaneous.jl:31

Probably a problem in StrideArraysCore?

ranocha commented 2 years ago

This happens only on Julia nightly, doesn't it?

ranocha commented 2 years ago

MWE using Julia v1.8.0-beta1:

   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.8.0-beta1 (2022-02-23)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(@v1.8) pkg> activate .
  Activating project at `~/.julia/dev/StrideArrays`

julia> using StrideArrays

julia> A = @StrideArray rand(10, 10); A_ptr = view(A, 1:5, 1:5)
5×5 StrideArray{Tuple{Int64, Int64}, (true, false), Float64, 2, 1, 0, (1, 2), Tuple{StaticInt{8}, StaticInt{80}}, Tuple{StaticInt{1}, StaticInt{1}}, StrideArraysCore.StaticStrideArray{Tuple{StaticInt{10}, StaticInt{10}}, (true, true), Float64, 2, 1, 0, (1, 2), Tuple{StaticInt{8}, StaticInt{80}}, Tuple{StaticInt{1}, StaticInt{1}}, 100}}:
 0.628102  0.400895  0.170133   0.507153  0.709167
 0.134677  0.26607   0.0299581  0.735177  0.957184
 0.679169  0.539174  0.450646   0.616162  0.504812
 0.199461  0.929504  0.412679   0.428801  0.183159
 0.199211  0.719763  0.30123    0.197397  0.750064

julia> mapreduce(abs2, max, A_ptr)
ERROR: StackOverflowError:
Stacktrace:
     [1] PtrArray
       @ ~/.julia/packages/StrideArraysCore/TzFER/src/ptr_array.jl:7 [inlined]
     [2] PtrArray
       @ ~/.julia/packages/StrideArraysCore/TzFER/src/ptr_array.jl:11 [inlined]
     [3] PtrArray
       @ ~/.julia/packages/StrideArraysCore/TzFER/src/ptr_array.jl:24 [inlined]
     [4] maybe_ptr_array
       @ ~/.julia/packages/StrideArraysCore/TzFER/src/stridearray.jl:150 [inlined]
     [5] maybe_ptr_array
       @ ~/.julia/packages/StrideArraysCore/TzFER/src/stridearray.jl:149 [inlined]
     [6] mapreduce(f::typeof(abs2), op::typeof(max), A::PtrArray{Tuple{Int64, Int64}, (true, false), Float64, 2, 1, 0, (1, 2), Tuple{StaticInt{8}, StaticInt{80}}, Tuple{StaticInt{1}, StaticInt{1}}})
       @ StrideArrays ~/.julia/dev/StrideArrays/src/miscellaneous.jl:31
     [7] vmapreduce(::typeof(abs2), ::typeof(max), ::PtrArray{Tuple{Int64, Int64}, (true, false), Float64, 2, 1, 0, (1, 2), Tuple{StaticInt{8}, StaticInt{80}}, Tuple{StaticInt{1}, StaticInt{1}}})
       @ LoopVectorization ~/.julia/packages/LoopVectorization/wLMFa/src/simdfunctionals/mapreduce.jl:37
--- the last 2 lines are repeated 39990 more times ---
 [79988] mapreduce(f::typeof(abs2), op::typeof(max), A::PtrArray{Tuple{Int64, Int64}, (true, false), Float64, 2, 1, 0, (1, 2), Tuple{StaticInt{8}, StaticInt{80}}, Tuple{StaticInt{1}, StaticInt{1}}})
       @ StrideArrays ~/.julia/dev/StrideArrays/src/miscellaneous.jl:31

mapreduce(abs2, max, A) and similar operations work fine - it seems to be specific to views (or similar arrays that are not contiguous in memory).

ranocha commented 2 years ago

This also happens on Julia v1.7.2 for me. In StrideArrays.jl,

https://github.com/chriselrod/StrideArrays.jl/blob/9e768b2a0897bd09f9071ff69692e922299076cc/src/miscellaneous.jl#L30-L32

calls

@inline function vmapreduce(f::F, op::OP, arg1::AbstractArray{T}, args::Vararg{AbstractArray{T},A}) where {F,OP,T<:NativeTypes,A}
    if !(check_args(arg1, args...) && all_dense(arg1, args...))
        return mapreduce(f, op, arg1, args...)
    end
...

https://github.com/JuliaSIMD/LoopVectorization.jl/blob/ed466fb1ca7e92b70b98d6ee50eb5544b64678e1/src/simdfunctionals/mapreduce.jl#L35-L38

from LoopVectorization.jl, which in turn calls the first method from StrideArrays.jl again.

ranocha commented 2 years ago

Any good idea how to break this cycle? Maybe it's too late over here but I do not see a good way to override Base.mapreduce for general AbstractStrideArrays while keeping Base.mapreduce available for non-dense AbstractStrideArrays. Does any of the type parameters of AbstractStrideArray{S,D,T,N,C,B,R,X,O} tell us whether it's all_dense?

chriselrod commented 2 years ago

Not exactly elegant, but a lazy workaround: https://github.com/chriselrod/StrideArrays.jl/pull/43

chriselrod commented 2 years ago

All tests passed in #43 (which only tried to fix the bug blocking this), except for the no-ambiguities check which found one with map and StaticArrays. Feel free to merge #43 into this and just disable all ambiguity checks as you do here, and I can merge, and then address that separate issue later.

ranocha commented 2 years ago

Sounds good to me :+1: I don't have merge right but you can go ahead

ranocha commented 2 years ago

@chriselrod I merged your PR #43 manually into this PR. Let's see whether CI passes this time

chriselrod commented 2 years ago

Sounds good to me 👍 I don't have merge right but you can go ahead

You shouldn't need that. git merge origin/avoidstackoverflow

ranocha commented 2 years ago

:+1:

You shouldn't need that. git merge origin/avoidstackoverflow

That's what I meant with

I merged your PR #43 manually into this PR. Let's see whether CI passes this time