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

Base.unaliascopy for SubArray is broken for indices with AbstractArray{<:CartesianIndex} #47644

Closed
jishnub opened this issue Nov 20, 2022 · 1 comment · Fixed by #47779
Closed
Labels
arrays [a, r, r, a, y, s] bug Indicates an unexpected problem or unintended behavior

Comments

@jishnub
Copy link
Contributor

jishnub commented Nov 20, 2022

julia> convert(CartesianIndex{1}, 1)
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type CartesianIndex{1}

Closest candidates are:
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:64
  CartesianIndex{N}(::Integer...) where N
   @ Base multidimensional.jl:72
  CartesianIndex{N}(::Integer...) where N
   @ Base multidimensional.jl:75

Stacktrace:
 [1] top-level scope
   @ REPL[1]:1

julia> convert(Int, CartesianIndex{1}(1)) # the other way works, so there's a correspondence
1

julia> CartesianIndex{1}(1) # a constructor is defined
CartesianIndex(1,)

julia> versioninfo()
Julia Version 1.10.0-DEV.12
Commit ee0f3fc334a (2022-11-16 00:27 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, tigerlake)
  Threads: 1 on 8 virtual cores
Environment:
  LD_LIBRARY_PATH = :/usr/lib/x86_64-linux-gnu/gtk-3.0/modules
  JULIA_EDITOR = subl

Perhaps the conversion from an Int to a CartesianIndex{1} should work as well.

Real-world case: https://discourse.julialang.org/t/using-views-with-cartesianindices-is-this-a-bug/19823/3

@brenhinkeller brenhinkeller added the arrays [a, r, r, a, y, s] label Nov 20, 2022
@N5N3
Copy link
Member

N5N3 commented Nov 21, 2022

Adding this convert seems ok to me.
But the root cause is that the Base.unaliascopy for SubArray is broken for indices with AbstractArray{<:CartesianIndex}

julia> a = randn(2,2)
2×2 Matrix{Float64}:
 -0.76243   -0.203595
  0.647722  -1.62516

julia> b = view(a, [CartesianIndex(2,2),CartesianIndex(1,1)])
2-element view(::Matrix{Float64}, CartesianIndex{2}[CartesianIndex(2, 2), CartesianIndex(1, 1)]) with eltype Float64:
 -1.6251623904863501
 -0.7624298422634336

julia> Base.unaliascopy(b)
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type CartesianIndex{2}

And I dont think we want to add convert from Int to CartesianIndex{N}.

@N5N3 N5N3 added the bug Indicates an unexpected problem or unintended behavior label Nov 21, 2022
@N5N3 N5N3 changed the title Missing convert from Int to CartesianIndex{1} Base.unaliascopy for SubArray is broken for indices with AbstractArray{<:CartesianIndex} Nov 21, 2022
N5N3 added a commit that referenced this issue Dec 11, 2023
…ex}` (#47779)

This PR fixes the bug caused by the trimming trick.
`Base.index_lengths` is not a proper tool to calculate the trimmed shape
as `indices` might consume more than 1 dim.
And we can avoid the unneeded "`repeat`" when we meet
`Array{CartesianIndex{0}}`

Test added.
Close #47644.
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] bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants