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

The IndexCartesian branch of copyto! is broken as _unsetindex is undefined for CartesianIndex arguments #53098

Closed
jishnub opened this issue Jan 29, 2024 · 1 comment · Fixed by #53383
Labels
arrays [a, r, r, a, y, s]

Comments

@jishnub
Copy link
Contributor

jishnub commented Jan 29, 2024

This is not a regression, but the situation may be improved. On julia nightly,

julia> M = Matrix{BigFloat}(undef, 2, 2)
2×2 Matrix{BigFloat}:
 #undef  #undef
 #undef  #undef

julia> copyto!(view(similar(M), axes(M)...), M)
ERROR: MethodError: no method matching _unsetindex!(::SubArray{BigFloat, 2, Matrix{BigFloat}, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}, false}, ::CartesianIndex{2})

Closest candidates are:
  _unsetindex!(::AbstractArray, ::Integer)
   @ Base abstractarray.jl:1447

Stacktrace:
 [1] copyto_unaliased!(deststyle::IndexCartesian, dest::SubArray{BigFloat, 2, Matrix{…}, Tuple{…}, false}, srcstyle::IndexLinear, src::Matrix{BigFloat})
   @ Base ./abstractarray.jl:1115
 [2] copyto!(dest::SubArray{BigFloat, 2, Matrix{BigFloat}, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}, false}, src::Matrix{BigFloat})
   @ Base ./abstractarray.jl:1058
 [3] top-level scope
   @ REPL[2]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> methods(Base._unsetindex!)
# 4 methods for generic function "_unsetindex!" from Base:
 [1] _unsetindex!(A::Array, i::Int64)
     @ array.jl:216
 [2] _unsetindex!(A::Memory, i::Int64)
     @ genericmemory.jl:33
 [3] _unsetindex!(A::AbstractArray, i::Integer)
     @ abstractarray.jl:1447
 [4] _unsetindex!(A::MemoryRef{T}) where T
     @ genericmemory.jl:34

The function _unsetindex! appears to be used in the IndexCartesian branch of copyto_unaliased!, but it's not defined for a CartesianIndex. Perhaps a new method should be added.

julia> versioninfo()
Julia Version 1.11.0-DEV.1397
Commit 0588cd40786 (2024-01-29 02:21 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, tigerlake)
Threads: 1 default, 0 interactive, 1 GC (on 8 virtual cores)
Environment:
  LD_LIBRARY_PATH = :/usr/lib/x86_64-linux-gnu/gtk-3.0/modules
  JULIA_EDITOR = subl
@jishnub jishnub added the arrays [a, r, r, a, y, s] label Jan 29, 2024
@jishnub jishnub changed the title The CartesianIndex branch of copyto! is broken as _unsetindex is undefined for CartesianIndex arguments The IndexCartesian branch of copyto! is broken as _unsetindex is undefined for CartesianIndex arguments Jan 29, 2024
@jishnub
Copy link
Contributor Author

jishnub commented Jan 29, 2024

There is also the following stack-overflow in the linear-indexing case:

julia> M = Matrix{BigFloat}(undef, 2, 2)
2×2 Matrix{BigFloat}:
 #undef  #undef
 #undef  #undef

julia> copyto!(view(similar(M), :, :), M)
ERROR: StackOverflowError:
Stacktrace:
 [1] _unsetindex!(A::SubArray{BigFloat, 2, Matrix{BigFloat}, Tuple{Base.Slice{Base.OneTo{Int64}}, Base.Slice{Base.OneTo{Int64}}}, true}, i::Int64) (repeats 79984 times)
   @ Base ./abstractarray.jl:1447

vtjnash pushed a commit that referenced this issue Feb 20, 2024
…3383)

With this, the following (and equivalent calls) work:
```julia
julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef
```

Close #53098. With this, all
the `_unsetindex!` branches in `copyto_unaliased!` work for
`Array`-views, and this makes certain indexing operations vectorize and
speed-up:
```julia
julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @Btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR
``` 

Improves (but doesn't resolve)
#40962 and
#53158

```julia
julia> a = rand(40,40); b = rand(40,40);

julia> @Btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR
```
ƒ
Co-authored-by: Jameson Nash <[email protected]>
tecosaur pushed a commit to tecosaur/julia that referenced this issue Mar 4, 2024
…liaLang#53383)

With this, the following (and equivalent calls) work:
```julia
julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef
```

Close JuliaLang#53098. With this, all
the `_unsetindex!` branches in `copyto_unaliased!` work for
`Array`-views, and this makes certain indexing operations vectorize and
speed-up:
```julia
julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @Btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR
``` 

Improves (but doesn't resolve)
JuliaLang#40962 and
JuliaLang#53158

```julia
julia> a = rand(40,40); b = rand(40,40);

julia> @Btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR
```
ƒ
Co-authored-by: Jameson Nash <[email protected]>
mkitti pushed a commit to mkitti/julia that referenced this issue Mar 7, 2024
…liaLang#53383)

With this, the following (and equivalent calls) work:
```julia
julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef
```

Close JuliaLang#53098. With this, all
the `_unsetindex!` branches in `copyto_unaliased!` work for
`Array`-views, and this makes certain indexing operations vectorize and
speed-up:
```julia
julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @Btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR
``` 

Improves (but doesn't resolve)
JuliaLang#40962 and
JuliaLang#53158

```julia
julia> a = rand(40,40); b = rand(40,40);

julia> @Btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR
```
ƒ
Co-authored-by: Jameson Nash <[email protected]>
KristofferC pushed a commit that referenced this issue Mar 27, 2024
…3383)

With this, the following (and equivalent calls) work:
```julia
julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef
```

Close #53098. With this, all
the `_unsetindex!` branches in `copyto_unaliased!` work for
`Array`-views, and this makes certain indexing operations vectorize and
speed-up:
```julia
julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @Btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR
```

Improves (but doesn't resolve)
#40962 and
#53158

```julia
julia> a = rand(40,40); b = rand(40,40);

julia> @Btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR
```
ƒ
Co-authored-by: Jameson Nash <[email protected]>

(cherry picked from commit 1a90409)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant