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

Deprecate cumsum, cumprod, cumsum_kbn, and accumulate when dim isn't specified #24684

Merged
merged 5 commits into from
Nov 24, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Deprecate cumsum, cumprod, cumsum_kbn, and accumulate when dim isn't
specified and dimension is larger than one
  • Loading branch information
andreasnoack committed Nov 23, 2017
commit 63d785164b25e5ece5bc685360fd2eb9c9843b88
6 changes: 3 additions & 3 deletions base/abstractarraymath.jl
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,12 @@ end
# TODO: Needs a separate IndexCartesian method, this is only fast for IndexLinear

"""
cumsum_kbn(A, [dim::Integer=1])
cumsum_kbn(A, dim::Integer)

Cumulative sum along a dimension, using the Kahan-Babuska-Neumaier compensated summation
algorithm for additional accuracy. The dimension defaults to 1.
algorithm for additional accuracy.
"""
function cumsum_kbn(A::AbstractArray{T}, axis::Integer=1) where T<:AbstractFloat
function cumsum_kbn(A::AbstractArray{T}, axis::Integer) where T<:AbstractFloat
dimsA = size(A)
ndimsA = ndims(A)
axis_size = dimsA[axis]
Expand Down
3 changes: 3 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2105,6 +2105,9 @@ end
@deprecate chol!(x::Number, uplo) chol(x) false
end

@deprecate cumsum(A::AbstractArray) cumsum(A, 1)
@deprecate cumsum_kbn(A::AbstractArray) cumsum_kbn(A, 1)
@deprecate cumprod(A::AbstractArray) cumprod(A, 1)

# issue #16307
@deprecate finalizer(o, f::Function) finalizer(f, o)
Expand Down
48 changes: 31 additions & 17 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ function accumulate_pairwise(op, v::AbstractVector{T}) where T
return accumulate_pairwise!(op, out, v)
end

function cumsum!(out, v::AbstractVector, axis::Integer=1)
function cumsum!(out, v::AbstractVector, axis::Integer)
# we dispatch on the possibility of numerical stability issues
_cumsum!(out, v, axis, TypeArithmetic(eltype(out)))
end
Expand All @@ -691,7 +691,7 @@ function _cumsum!(out, v, axis, ::TypeArithmetic)
end

"""
cumsum(A, dim=1)
cumsum(A, dim)
Copy link
Member

Choose a reason for hiding this comment

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

AFAICT there should be a separate docstring for AbstractVector, right? Also, ::Integer would be useful since there's no longer any indication of the expected type in the signature.


Cumulative sum along a dimension `dim`. See also [`cumsum!`](@ref)
to use a preallocated output array, both for performance and to control the precision of the
Expand All @@ -714,20 +714,24 @@ julia> cumsum(a,2)
4 9 15
```
"""
function cumsum(A::AbstractArray{T}, axis::Integer=1) where T
function cumsum(A::AbstractArray{T}, axis::Integer) where T
out = similar(A, rcum_promote_type(+, T))
cumsum!(out, A, axis)
end

cumsum(x::AbstractVector) = cumsum(x, 1)

"""
cumsum!(B, A, dim::Integer=1)
cumsum!(B, A, dim::Integer)

Cumulative sum of `A` along a dimension, storing the result in `B`. See also [`cumsum`](@ref).
"""
cumsum!(B, A, axis::Integer=1) = accumulate!(+, B, A, axis)
cumsum!(B, A, axis::Integer) = accumulate!(+, B, A, axis)

cumsum!(y::AbstractVector, x::AbstractVector) = cumsum!(y, x, 1)

"""
cumprod(A, dim=1)
cumprod(A, dim)

Cumulative product along a dimension `dim`. See also
[`cumprod!`](@ref) to use a preallocated output array, both for performance and
Expand All @@ -750,18 +754,22 @@ julia> cumprod(a,2)
4 20 120
```
"""
cumprod(A::AbstractArray, axis::Integer=1) = accumulate(*, A, axis)
cumprod(A::AbstractArray, axis::Integer) = accumulate(*, A, axis)

cumprod(x::AbstractVector) = cumprod(x, 1)

"""
cumprod!(B, A, dim::Integer=1)
cumprod!(B, A, dim::Integer)

Cumulative product of `A` along a dimension, storing the result in `B`.
See also [`cumprod`](@ref).
"""
cumprod!(B, A, axis::Integer=1) = accumulate!(*, B, A, axis)
cumprod!(B, A, axis::Integer) = accumulate!(*, B, A, axis)

cumprod!(y::AbstractVector, x::AbstractVector) = cumprod!(y, x, 1)

"""
accumulate(op, A, dim=1)
accumulate(op, A, dim)

Cumulative operation `op` along a dimension `dim`. See also
[`accumulate!`](@ref) to use a preallocated output array, both for performance and
Expand All @@ -783,11 +791,12 @@ julia> accumulate(*, [1,2,3])
6
```
"""
function accumulate(op, A, axis::Integer=1)
function accumulate(op, A, axis::Integer)
out = similar(A, rcum_promote_type(op, eltype(A)))
accumulate!(op, out, A, axis)
end

accumulate(op, x::AbstractVector) = accumulate(op, x, 1)

"""
accumulate(op, v0, A)
Expand All @@ -810,26 +819,29 @@ julia> accumulate(min, 0, [1,2,-1])
-1
```
"""
function accumulate(op, v0, A, axis::Integer=1)
function accumulate(op, v0, A, axis::Integer)
T = rcum_promote_type(op, typeof(v0), eltype(A))
out = similar(A, T)
accumulate!(op, out, v0, A, 1)
end

function accumulate!(op::Op, B, A::AbstractVector, axis::Integer=1) where Op
accumulate(op, v0, x::AbstractVector) = accumulate(op, v0, x, 1)

function accumulate!(op::Op, B, A::AbstractVector, axis::Integer) where Op
isempty(A) && return B
v1 = first(A)
_accumulate1!(op, B, v1, A, axis)
end

function accumulate!(op, B, v0, A::AbstractVector, axis::Integer=1)
function accumulate!(op, B, v0, A::AbstractVector, axis::Integer)
isempty(A) && return B
v1 = op(v0, first(A))
_accumulate1!(op, B, v1, A, axis)
end

accumulate!(op, y::AbstractVector, v0, x::AbstractVector) = accumulate!(op, y, v0, x, 1)

function _accumulate1!(op, B, v1, A::AbstractVector, axis::Integer=1)
function _accumulate1!(op, B, v1, A::AbstractVector, axis::Integer)
axis > 0 || throw(ArgumentError("axis must be a positive integer"))
inds = linearindices(A)
inds == linearindices(B) || throw(DimensionMismatch("linearindices of A and B don't match"))
Expand All @@ -845,12 +857,12 @@ function _accumulate1!(op, B, v1, A::AbstractVector, axis::Integer=1)
end

"""
accumulate!(op, B, A, dim=1)
accumulate!(op, B, A, dim)

Cumulative operation `op` on `A` along a dimension, storing the result in `B`.
See also [`accumulate`](@ref).
"""
function accumulate!(op, B, A, axis::Integer=1)
function accumulate!(op, B, A, axis::Integer)
axis > 0 || throw(ArgumentError("axis must be a positive integer"))
inds_t = indices(A)
indices(B) == inds_t || throw(DimensionMismatch("shape of B must match A"))
Expand All @@ -876,6 +888,8 @@ function accumulate!(op, B, A, axis::Integer=1)
return B
end

accumulate!(op, y::AbstractVector, x::AbstractVector) = accumulate!(op, y, x, 1)

@noinline function _accumulate!(op, B, A, R1, ind, R2)
# Copy the initial element in each 1d vector along dimension `axis`
ii = first(ind)
Expand Down
3 changes: 1 addition & 2 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,6 @@ end
A1 = reshape(repmat([1,2],1,12),2,3,4)
A2 = reshape(repmat([1 2 3],2,4),2,3,4)
A3 = reshape(repmat([1 2 3 4],6,1),2,3,4)
@test isequal(cumsum(A),A1)
@test isequal(cumsum(A,1),A1)
@test isequal(cumsum(A,2),A2)
@test isequal(cumsum(A,3),A3)
Expand Down Expand Up @@ -931,7 +930,7 @@ end
end

# issue #2342
@test isequal(cumsum([1 2 3]), [1 2 3])
@test isequal(cumsum([1 2 3], 1), [1 2 3])

@testset "set-like operations" begin
@test isequal(union([1,2,3], [4,3,4]), [1,2,3,4])
Expand Down