JuliaArrays / ArrayInterface.jl

Designs for new Base array interface primitives, used widely through scientific machine learning (SciML) and other organizations
https://docs.sciml.ai/ArrayInterface/stable/
MIT License
134 stars 37 forks source link

Strides on reshaped views wrong #318

Closed chriselrod closed 2 years ago

chriselrod commented 2 years ago
julia> using ArrayInterface

julia> A = reshape(view(rand(17,5), 1:16, :), (4,4,:));

julia> strides(A)
ERROR: ArgumentError: Parent must be contiguous.
Stacktrace:
 [1] strides(a::Base.ReshapedArray{Float64, 3, SubArray{Float64, 2, Matrix{Float64}, Tuple{UnitRange{Int64}, Base.Slice{Base.OneTo{Int64}}}, false}, Tuple{Base.MultiplicativeInverses.SignedMultiplicativeInverse{Int64}}})
   @ Base ./reshapedarray.jl:305
 [2] top-level scope
   @ REPL[3]:1

julia> ArrayInterface.strides(A)
(static(1), 4, 16)

The answer should be static(1), 4, 17.

Given that we have

julia> ArrayInterface.strides(parent(A))
(static(1), 17)

it looks like reshape isn't looking to the parent's strides.

Tokazama commented 2 years ago

I think @N5N3 wrote the most recent code for that.

N5N3 commented 2 years ago

~Looks like the column dense check false passed here. I’ll take a look tomorrow.~ I can't reproduce localy (with ArrayInterface 6.0.17)

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

(@v1.7) pkg> activate test
  Activating new project at `C:\Users\MYJ\test`

(test) pkg> add ArrayInterface
    Updating registry at `C:\Users\MYJ\.julia\registries\General`
    Updating git-repo `https://github.com/JuliaRegistries/General.git`
   Resolving package versions...
   Installed Compat ─ v4.1.0
    Updating `C:\Users\MYJ\test\Project.toml`
  [4fba245c] + ArrayInterface v6.0.17
    Updating `C:\Users\MYJ\test\Manifest.toml`
  [4fba245c] + ArrayInterface v6.0.17
  [30b0a656] + ArrayInterfaceCore v0.1.12
  [34da2185] + Compat v4.1.0
  [615f187c] + IfElse v0.1.1
  [aedffcd0] + Static v0.6.6
  [56f22d72] + Artifacts
  [ade2ca70] + Dates
  [8f399da3] + Libdl
  [37e2e46d] + LinearAlgebra
  [de0858da] + Printf
  [9a3f8284] + Random
  [ea8e919c] + SHA
  [9e88b42a] + Serialization
  [2f01184e] + SparseArrays
  [4607b0f0] + SuiteSparse
  [cf7118a7] + UUIDs
  [4ec0a83e] + Unicode
  [e66e0078] + CompilerSupportLibraries_jll
  [4536629a] + OpenBLAS_jll
  [8e850b90] + libblastrampoline_jll
Precompiling project...
  4 dependencies successfully precompiled in 2 seconds (4 already precompiled)

julia> using ArrayInterface

julia> A = reshape(view(randn(17,5), 1:16, :), 4, 4, :);

julia> ArrayInterface.strides(A)
(1, 4, 17)

julia> B = reshape(view(randn(2,5*16), 1:1, :), 4, 4, :);

julia> ArrayInterface.strides(B)
(2, 8, 32)

julia> typeof(B) == typeof(A)
true
chriselrod commented 2 years ago

Oops. I was on an old version of ArrayInterface.