search

meggart / DiskArrays.jl

Other
79 stars 16 forks source link

bugfix with JET #77

Closed rafaqz closed 2 years ago

rafaqz commented 2 years ago

This is from running JET after ] activate . in the DiskArrays.jl environment, with:

julia> JET.report_package(DiskArrays)

This PR cleans up most of the bugs, although some would be good to have tests as well. Still remaining are the expected "may throw" and some weirder deeply nested errors in Base, that seem to be problems in Base or JET more than here:

═════ 5 possible errors found ═════
┌ @ /home/raf/.julia/dev/DiskArrays/src/diskarray.jl:27 DiskArrays.batchgetindex(tuple(a), i...)
│┌ @ /home/raf/.julia/dev/DiskArrays/src/batchgetindex.jl:68 DiskArrays.batchgetindex(a, ci[i])
││┌ @ /home/raf/.julia/dev/DiskArrays/src/batchgetindex.jl:72 DiskArrays.disk_getindex_batch(a, indvec)
│││┌ @ /home/raf/.julia/dev/DiskArrays/src/batchgetindex.jl:119 prep = DiskArrays.prepare_disk_getindex_batch(ar, indstoread)
││││┌ @ /home/raf/.julia/dev/DiskArrays/src/batchgetindex.jl:78 inds = DiskArrays.collect(DiskArrays.Any, Base.OneTo(N))
│││││┌ @ array.jl:647 Base._collect(T, itr, Base.IteratorSize(itr))
││││││┌ @ array.jl:649 Base._array_for(T, isz, Base._similar_shape(itr, isz))
│││││││┌ @ array.jl:676 similar(Vector{Any}, axs)
││││││││┌ @ abstractarray.jl:840 similar(T, Base.to_shape(shape))
│││││││││┌ @ /home/raf/.julia/packages/OffsetArrays/WvkHl/src/OffsetArrays.jl:334 OffsetArrays.map(OffsetArrays._offset, OffsetArrays.axes(P), shape)
││││││││││┌ @ tuple.jl:250 map(f, Base.tail(t), Base.tail(s))
│││││││││││┌ @ tuple.jl:250 s[1]
││││││││││││┌ @ tuple.jl:29 Base.getfield(t, i, $(Expr(:boundscheck)))
│││││││││││││ invalid builtin function call
││││││││││││└───────────────
┌ @ /home/raf/.julia/dev/DiskArrays/src/diskarray.jl:65 DiskArrays.interpret_indices_disk(::Any, ::Tuple)
│ may throw: DiskArrays.throw(DiskArrays.ArgumentError(string("Indices of type ", DiskArrays.typeof(r::Tuple)::Type{<:Tuple}, " are not yet supported")::String)::ArgumentErr
or)::Union{}
└───────────────────────────────────────────────────────
┌ @ /home/raf/.julia/dev/DiskArrays/src/chunks.jl:84 throw(UndefKeywordError(:chunksizes))
│ UndefKeywordError: keyword argument chunksizes not assigned: throw(UndefKeywordError(:chunksizes)::UndefKeywordError)::Union{}
└────────────────────────────────────────────────────
┌ @ /home/raf/.julia/dev/DiskArrays/src/batchgetindex.jl:174 Base.materialize!(A_ret, Base.broadcasted(identity, DiskArrays.batchgetindex(tuple(A), r...)))
│┌ @ broadcast.jl:1175 val = Base.Broadcast.view(B.parent, B.mask)
││┌ @ subarray.jl:176 J = map(#185, to_indices(A, I))
│││┌ @ tuple.jl:221 f(t[1])
││││┌ @ subarray.jl:176 Base.unalias(getfield(#self#, :A), i)
│││││┌ @ abstractarray.jl:1427 Base.unaliascopy(A)
││││││┌ @ abstractarray.jl:1018 Base.copyto_unaliased!(IndexStyle(dest), dest, IndexStyle(src′), src′)
│││││││┌ @ abstractarray.jl:1043  = iterate(src)
││││││││┌ @ multidimensional.jl:778 iterate(L, tuple(1, r))
│││││││││┌ @ multidimensional.jl:811 Base.indexed_iterate(_3, 3, _5)
││││││││││┌ @ tuple.jl:88 Base.getfield(t, i)
│││││││││││ invalid builtin function call
││││││││││└───────────────
│││││││┌ @ abstractarray.jl:1045  = iterate(src, getfield(_9, 2))
││││││││┌ @ multidimensional.jl:811 Base.indexed_iterate(_3, 4, _5)
│││││││││┌ @ tuple.jl:88 Base.getfield(t, i)
││││││││││ invalid builtin function call
│││││││││└───────────────
rafaqz commented 2 years ago

It seems quite amazing. It even turned out one of the errors above is a bug in Base that they have since fixed.

I'm running it manually on a bunch of my packages and JuliaGeo things. But it might be nice to have the output printed in CI logs so we can check it routinely on PRs.