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

RFC: make reshape always share data #15916

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,12 @@ Deprecated or removed

* `scale` is deprecated in favor of either `α*A`, `Diagonal(x)*A`, or `A*Diagonal(x)`. ([#15258])

* `reshape` is now defined to always share data with the original array.
Uses like `reshape(1:10, (2,5))` should be replaced with `collect(1:10, (2,5))` ([#4211]).

[PkgDev]: https://github.com/JuliaLang/PkgDev.jl
<!--- generated by NEWS-update.jl: -->
[#4211]: https://github.com/JuliaLang/julia/issues/4211
[#8036]: https://github.com/JuliaLang/julia/issues/8036
[#8846]: https://github.com/JuliaLang/julia/issues/8846
[#9503]: https://github.com/JuliaLang/julia/issues/9503
Expand Down Expand Up @@ -190,6 +194,7 @@ Deprecated or removed
[#15192]: https://github.com/JuliaLang/julia/issues/15192
[#15242]: https://github.com/JuliaLang/julia/issues/15242
[#15258]: https://github.com/JuliaLang/julia/issues/15258
[#15409]: https://github.com/JuliaLang/julia/issues/15409
[#15550]: https://github.com/JuliaLang/julia/issues/15550
[#15609]: https://github.com/JuliaLang/julia/issues/15609
[#15763]: https://github.com/JuliaLang/julia/issues/15763
6 changes: 0 additions & 6 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -198,12 +198,6 @@ similar( a::AbstractArray, T::Type, dims::Integer...) = similar(a, T, dims)
similar( a::AbstractArray, T::Type, dims::DimsInteger) = Array(T, dims...)
similar( a::AbstractArray, T::Type, dims::Dims) = Array(T, dims)

function reshape(a::AbstractArray, dims::Dims)
if prod(dims) != length(a)
throw(ArgumentError("dimensions must be consistent with array size (expected $(length(a)), got $(prod(dims)))"))
end
copy!(similar(a, dims), a)
end
reshape(a::AbstractArray, dims::Int...) = reshape(a, dims)

## from general iterable to any array
Expand Down
17 changes: 13 additions & 4 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,25 @@ _similar_for(c::AbstractArray, T, itr, ::HasShape) = similar(c, T, convert(Dims,
_similar_for(c, T, itr, isz) = similar(c, T)

"""
collect(element_type, collection)
collect(element_type, iterator)

Return an array of type `Array{element_type,1}` of all items in a collection.
Return an array of type `Array{element_type,1}` of all items from an iterator.
"""
collect{T}(::Type{T}, itr) = collect(Generator(T, itr))

collect{T}(::Type{T}, itr::Dims) = collect(Generator(T, itr))

"""
collect(iterator, dims)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs an rst signature and a genstdlib run


Return an array with the given dimensions of all elements from an iterator.
"""
collect(itr, dims::Dims) = collect(IteratorND(itr, dims))

"""
collect(collection)
collect(iterator)

Return an array of all items in a collection. For associative collections, returns Pair{KeyType, ValType}.
Return an array of all items from an iterator. For associative collections, returns Pair{KeyType, ValType}.
"""
collect(itr) = _collect(1:1 #= Array =#, itr, iteratoreltype(itr), iteratorsize(itr))

Expand Down
5 changes: 5 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1037,3 +1037,8 @@ function pmap(f, c...; err_retry=nothing, err_stop=nothing, pids=nothing)

return pmap(p, f, c...)
end

# 4211
@deprecate reshape(a::AbstractArray, dims::Dims) copy!(similar(a, dims), a)
@deprecate reshape{Tv,Ti}(a::SparseMatrixCSC{Tv,Ti}, dims::NTuple{2,Int}) copy!(similar(a, dims), a)
@deprecate reshape(r::Range, dims::Dims) collect(r, dims)
4 changes: 1 addition & 3 deletions base/docs/helpdb/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1572,9 +1572,7 @@ ind2chr
"""
reshape(A, dims)

Create an array with the same data as the given array, but with different dimensions. An
implementation for a particular type of array may choose whether the data is copied or
shared.
Create an array with the same data as the given array, but with different dimensions.
"""
reshape

Expand Down
18 changes: 2 additions & 16 deletions base/sparse/sparsematrix.jl
Original file line number Diff line number Diff line change
Expand Up @@ -172,22 +172,6 @@ function reinterpret{T,Tv,Ti,N}(::Type{T}, a::SparseMatrixCSC{Tv,Ti}, dims::NTup
return SparseMatrixCSC(mS, nS, colptr, rowval, nzval)
end

function reshape{Tv,Ti}(a::SparseMatrixCSC{Tv,Ti}, dims::NTuple{2,Int})
if prod(dims) != length(a)
throw(DimensionMismatch("new dimensions $(dims) must be consistent with array size $(length(a))"))
end
mS,nS = dims
mA,nA = size(a)
numnz = nnz(a)
colptr = Array(Ti, nS+1)
rowval = similar(a.rowval)
nzval = copy(a.nzval)

sparse_compute_reshaped_colptr_and_rowval(colptr, rowval, mS, nS, a.colptr, a.rowval, mA, nA)

return SparseMatrixCSC(mS, nS, colptr, rowval, nzval)
end

## Constructors

copy(S::SparseMatrixCSC) =
Expand Down Expand Up @@ -2933,6 +2917,8 @@ function blkdiag(X::SparseMatrixCSC...)
SparseMatrixCSC(m, n, colptr, rowval, nzval)
end

squeeze(S::SparseMatrixCSC, dims::Dims) = throw(ArgumentError("squeeze is not available for sparse matrices"))

## Structure query functions
issymmetric(A::SparseMatrixCSC) = is_hermsym(A, IdFun())

Expand Down
19 changes: 8 additions & 11 deletions test/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ import Base: trailingsize
const can_inline = Base.JLOptions().can_inline != 0
function test_scalar_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)
@test A == B
# Test indexing up to 5 dimensions
Expand Down Expand Up @@ -186,23 +186,23 @@ end

function test_vector_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)
idxs = rand(1:N, 3, 3, 3)
@test B[idxs] == A[idxs] == idxs
@test B[vec(idxs)] == A[vec(idxs)] == vec(idxs)
@test B[:] == A[:] == collect(1:N)
@test B[1:end] == A[1:end] == collect(1:N)
@test B[:,:] == A[:,:] == reshape(1:N, shape[1], prod(shape[2:end]))
@test B[1:end,1:end] == A[1:end,1:end] == reshape(1:N, shape[1], prod(shape[2:end]))
@test B[:,:] == A[:,:] == collect(1:N, (shape[1], prod(shape[2:end])))
@test B[1:end,1:end] == A[1:end,1:end] == collect(1:N, (shape[1], prod(shape[2:end])))
# Test with containers that aren't Int[]
@test B[[]] == A[[]] == []
@test B[convert(Array{Any}, idxs)] == A[convert(Array{Any}, idxs)] == idxs
end

function test_primitives{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)

# last(a)
Expand All @@ -221,9 +221,6 @@ function test_primitives{T}(::Type{T}, shape, ::Type{TestAbstractArray})
@test isassigned(B, length(B) + 1) == false
end

# reshape(a::AbstractArray, dims::Dims)
@test_throws ArgumentError reshape(B, (0, 1))

# copy!(dest::AbstractArray, src::AbstractArray)
@test_throws BoundsError copy!(Array(Int, 10), [1:11...])

Expand Down Expand Up @@ -252,7 +249,7 @@ type UnimplementedArray{T, N} <: AbstractArray{T, N} end

function test_getindex_internals{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)

@test getindex(A) == 1
Expand All @@ -272,7 +269,7 @@ end

function test_setindex!_internals{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)

Base.unsafe_setindex!(B, 1)
Expand Down Expand Up @@ -362,7 +359,7 @@ function test_ind2sub(::Type{TestAbstractArray})
n = rand(2:5)
dims = tuple(rand(1:5, n)...)
len = prod(dims)
A = reshape(1:len, dims...)
A = collect(1:len, dims)
I = ind2sub(dims, [1:len...])
for i in 1:len
idx = [ I[j][i] for j in 1:n ]
Expand Down
17 changes: 8 additions & 9 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ a = rand(1, 1, 8, 8, 1)
@test_throws ArgumentError squeeze(a, 6)

sz = (5,8,7)
A = reshape(1:prod(sz),sz...)
A = collect(1:prod(sz), sz)
@test A[2:6] == [2:6;]
@test A[1:3,2,2:4] == cat(2,46:48,86:88,126:128)
@test A[:,7:-3:1,5] == [191 176 161; 192 177 162; 193 178 163; 194 179 164; 195 180 165]
@test A[:,3:9] == reshape(11:45,5,7)
@test A[:,3:9] == collect(11:45, (5,7))
rng = (2,2:3,2:2:5)
tmp = zeros(Int,map(maximum,rng)...)
tmp[rng...] = A[rng...]
Expand Down Expand Up @@ -138,7 +138,7 @@ B[[3,1],[2,4]] = [21 22; 23 24]
B[4,[2,3]] = 7
@test B == [0 23 1 24 0; 11 12 13 14 15; 0 21 3 22 0; 0 7 7 0 0]

@test isequal(reshape(1:27, 3, 3, 3)[1,:], [1, 4, 7, 10, 13, 16, 19, 22, 25])
@test isequal(collect(1:27, (3, 3, 3))[1,:], [1, 4, 7, 10, 13, 16, 19, 22, 25])

a = [3, 5, -7, 6]
b = [4, 6, 2, -7, 1]
Expand Down Expand Up @@ -169,7 +169,7 @@ rt = Base.return_types(setindex!, Tuple{Array{Int32, 3}, UInt8, Vector{Int}, Int

# get
let
A = reshape(1:24, 3, 8)
A = collect(1:24, (3, 8))
x = get(A, 32, -12)
@test x == -12
x = get(A, 14, -12)
Expand Down Expand Up @@ -368,7 +368,7 @@ p = permutedims(s, [2,1])
@test p[2,1]==a[2,3] && p[2,2]==a[3,3]

# of a non-strided subarray
a = reshape(1:60, 3, 4, 5)
a = collect(1:60, (3, 4, 5))
s = sub(a,:,[1,2,4],[1,5])
c = convert(Array, s)
for p in ([1,2,3], [1,3,2], [2,1,3], [2,3,1], [3,1,2], [3,2,1])
Expand Down Expand Up @@ -553,7 +553,7 @@ let
3 3 4 4 3 3 4 4;
3 3 4 4 3 3 4 4]

A = reshape(1:8, 2, 2, 2)
A = collect(1:8, (2, 2, 2))
R = repeat(A, inner = [1, 1, 2], outer = [1, 1, 1])
T = reshape([1:4; 1:4; 5:8; 5:8], 2, 2, 4)
@test R == T
Expand Down Expand Up @@ -623,7 +623,7 @@ a[[true,false,true,false,true]] = 6
a[[true,false,true,false,true]] = [7,8,9]
@test a == [7,2,8,4,9]
@test_throws DimensionMismatch (a[[true,false,true,false,true]] = [7,8,9,10])
A = reshape(1:15, 3, 5)
A = collect(1:15, (3, 5))
@test A[[true, false, true], [false, false, true, true, false]] == [7 10; 9 12]
@test_throws BoundsError A[[true, false], [false, false, true, true, false]]
@test_throws BoundsError A[[true, false, true], [false, true, true, false]]
Expand Down Expand Up @@ -1092,7 +1092,7 @@ a = ones(5,0)
b = sub(a, :, :)
@test mdsum(b) == 0

a = reshape(1:60, 3, 4, 5)
a = collect(1:60, (3, 4, 5))
@test a[CartesianIndex{3}(2,3,4)] == 44
a[CartesianIndex{3}(2,3,3)] = -1
@test a[CartesianIndex{3}(2,3,3)] == -1
Expand Down Expand Up @@ -1353,7 +1353,6 @@ C = copy(B)
@test A == C
@test B == C

@test vec(A) == vec(B) == vec(S)
@test minimum(A) == minimum(B) == minimum(S)
@test maximum(A) == maximum(B) == maximum(S)

Expand Down
2 changes: 1 addition & 1 deletion test/dict.jl
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ end
# show
for d in (Dict("\n" => "\n", "1" => "\n", "\n" => "2"),
[string(i) => i for i = 1:30],
[reshape(1:i^2,i,i) => reshape(1:i^2,i,i) for i = 1:24],
[collect(1:i^2,(i,i)) => collect(1:i^2,(i,i)) for i = 1:24],
[utf8(Char['α':'α'+i;]) => utf8(Char['α':'α'+i;]) for i = (1:10)*10],
Dict("key" => zeros(0, 0)))
for cols in (12, 40, 80), rows in (2, 10, 24)
Expand Down
4 changes: 2 additions & 2 deletions test/examples.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ y = inv(x)

include(joinpath(dir, "ndgrid.jl"))
r = repmat(1:10,1,10)
r1, r2 = ndgrid(1:10, 1:10)
r1, r2 = ndgrid(collect(1:10), collect(1:10))
@test r1 == r
@test r2 == r'
r3, r4 = meshgrid(1:10,1:10)
r3, r4 = meshgrid(collect(1:10),collect(1:10))
@test r3 == r'
@test r4 == r

Expand Down
2 changes: 1 addition & 1 deletion test/fft.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ b = rand(17,14)
b[3:6,9:12] = m4
sm4 = slice(b,3:6,9:12)

m3d = map(Float32,reshape(1:5*3*2, 5, 3, 2))
m3d = map(Float32,collect(1:5*3*2, (5, 3, 2)))
true_fftd3_m3d = Array(Float32, 5, 3, 2)
true_fftd3_m3d[:,:,1] = 17:2:45
true_fftd3_m3d[:,:,2] = -15
Expand Down
2 changes: 1 addition & 1 deletion test/functional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ end
@test collect((i+10j for i=1:2,j=3:4,k=1:1)) == reshape([31 41; 32 42], (2,2,1))

let I = Base.IteratorND(1:27,(3,3,3))
@test collect(I) == reshape(1:27,(3,3,3))
@test collect(I) == collect(1:27,(3,3,3))
@test size(I) == (3,3,3)
@test length(I) == 27
@test eltype(I) === Int
Expand Down
10 changes: 5 additions & 5 deletions test/linalg/matmul.jl
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ C = zeros(Float64,6,6)
@test Base.LinAlg.Ac_mul_B!(C,A,B) == A.'*B

# matrix algebra with subarrays of floats (stride != 1)
A = reshape(map(Float64,1:20),5,4)
A = collect(1.0:20.0, (5,4))
Aref = A[1:2:end,1:2:end]
Asub = sub(A, 1:2:5, 1:2:4)
b = [1.2,-2.5]
Expand All @@ -127,23 +127,23 @@ Asub = sub(Ai, 1:2:5, 1:2:4)
@test Ac_mul_B(Asub, Asub) == Ac_mul_B(Aref, Aref)
@test A_mul_Bc(Asub, Asub) == A_mul_Bc(Aref, Aref)
# issue #15286
let C = zeros(8, 8), sC = sub(C, 1:2:8, 1:2:8), B = reshape(map(Float64,-9:10),5,4)
let C = zeros(8, 8), sC = sub(C, 1:2:8, 1:2:8), B = collect(-9.0:10.0, (5,4))
@test At_mul_B!(sC, A, A) == A'*A
@test At_mul_B!(sC, A, B) == A'*B
end
let Aim = A .- im, C = zeros(Complex128,8,8), sC = sub(C, 1:2:8, 1:2:8), B = reshape(map(Float64,-9:10),5,4) .+ im
let Aim = A .- im, C = zeros(Complex128,8,8), sC = sub(C, 1:2:8, 1:2:8), B = collect(-9.0:10.0, (5,4)) .+ im
@test Ac_mul_B!(sC, Aim, Aim) == Aim'*Aim
@test Ac_mul_B!(sC, Aim, B) == Aim'*B
end


# syrk & herk
A = reshape(1:1503, 501, 3).-750.0
A = collect(1:1503, (501, 3)).-750.0
res = Float64[135228751 9979252 -115270247; 9979252 10481254 10983256; -115270247 10983256 137236759]
@test At_mul_B(A, A) == res
@test A_mul_Bt(A',A') == res
cutoff = 501
A = reshape(1:6*cutoff,2*cutoff,3).-(6*cutoff)/2
A = collect(1:6*cutoff, (2*cutoff,3)).-(6*cutoff)/2
Asub = sub(A, 1:2:2*cutoff, 1:3)
Aref = A[1:2:2*cutoff, 1:3]
@test At_mul_B(Asub, Asub) == At_mul_B(Aref, Aref)
Expand Down
2 changes: 1 addition & 1 deletion test/parallel_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ d = SharedArray(Float64, (2,3))
fn = tempname()
write(fn, 1:30)
sz = (6,5)
Atrue = reshape(1:30, sz)
Atrue = collect(1:30, sz)

S = SharedArray(fn, Int, sz)
@test S == Atrue
Expand Down
12 changes: 6 additions & 6 deletions test/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
@test sum([3]) === 3
@test sum([3.0]) === 3.0

z = reshape(1:16, (2,2,2,2))
z = collect(1:16, (2,2,2,2))
fz = float(z)
@test sum(z) === 136
@test sum(fz) === 136.0
Expand Down Expand Up @@ -167,9 +167,9 @@ prod2(itr) = invoke(prod, Tuple{Any}, itr)
@test maximum(collect(Int16(1):Int16(100))) === Int16(100)
@test maximum(Int32[1,2]) === Int32(2)

@test extrema(reshape(1:24,2,3,4),1) == reshape([(1,2),(3,4),(5,6),(7,8),(9,10),(11,12),(13,14),(15,16),(17,18),(19,20),(21,22),(23,24)],1,3,4)
@test extrema(reshape(1:24,2,3,4),2) == reshape([(1,5),(2,6),(7,11),(8,12),(13,17),(14,18),(19,23),(20,24)],2,1,4)
@test extrema(reshape(1:24,2,3,4),3) == reshape([(1,19),(2,20),(3,21),(4,22),(5,23),(6,24)],2,3,1)
@test extrema(collect(1:24,(2,3,4)),1) == reshape([(1,2),(3,4),(5,6),(7,8),(9,10),(11,12),(13,14),(15,16),(17,18),(19,20),(21,22),(23,24)],1,3,4)
@test extrema(collect(1:24,(2,3,4)),2) == reshape([(1,5),(2,6),(7,11),(8,12),(13,17),(14,18),(19,23),(20,24)],2,1,4)
@test extrema(collect(1:24,(2,3,4)),3) == reshape([(1,19),(2,20),(3,21),(4,22),(5,23),(6,24)],2,3,1)

# any & all

Expand Down Expand Up @@ -275,11 +275,11 @@ end
@test sum(collect(map(UInt8,0:255))) == 32640
@test sum(collect(map(UInt8,254:255))) == 509

A = reshape(map(UInt8, 101:109), (3,3))
A = collect(UInt8(101):UInt8(109), (3,3))
@test @inferred(sum(A)) == 945
@test @inferred(sum(sub(A, 1:3, 1:3))) == 945

A = reshape(map(UInt8, 1:100), (10,10))
A = collect(UInt8(1):UInt8(100), (10,10))
@test @inferred(sum(A)) == 5050
@test @inferred(sum(sub(A, 1:10, 1:10))) == 5050

Expand Down
Loading