From 5ecd1cf90cd84c815668f6574a3c881ea8b57856 Mon Sep 17 00:00:00 2001 From: Kristoffer Carlsson Date: Fri, 21 Feb 2020 09:02:45 +0100 Subject: [PATCH] remove trim option to fkeep! and related functions (#32972) * remove trim option to fkeep! and related functions * add NEWS entry --- NEWS.md | 4 ++ stdlib/SparseArrays/src/sparsematrix.jl | 53 ++++++++++-------------- stdlib/SparseArrays/src/sparsevector.jl | 38 +++++++---------- stdlib/SparseArrays/test/sparse.jl | 4 -- stdlib/SparseArrays/test/sparsevector.jl | 4 -- 5 files changed, 40 insertions(+), 63 deletions(-) diff --git a/NEWS.md b/NEWS.md index af694dac0ba39..8b02c77758e3a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -91,6 +91,10 @@ Standard library changes #### SparseArrays * `lu!` accepts `UmfpackLU` as an argument to make use of its symbolic factorization. +* The `trim` keyword argument for the functions `fkeep!`, `tril!`, `triu!`, + `droptol!`,`dropzeros!` and `dropzeros` has been removed in favour of always + trimming. Calling these with `trim=false` could result in invalid sparse + arrays. #### Dates diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl index e453f5d21dcf2..de1f5e1430ad6 100644 --- a/stdlib/SparseArrays/src/sparsematrix.jl +++ b/stdlib/SparseArrays/src/sparsematrix.jl @@ -1274,7 +1274,7 @@ end ## fkeep! and children tril!, triu!, droptol!, dropzeros[!] """ - fkeep!(A::AbstractSparseArray, f, trim::Bool = true) + fkeep!(A::AbstractSparseArray, f) Keep elements of `A` for which test `f` returns `true`. `f`'s signature should be @@ -1283,8 +1283,7 @@ Keep elements of `A` for which test `f` returns `true`. `f`'s signature should b where `i` and `j` are an element's row and column indices and `x` is the element's value. This method makes a single sweep through `A`, requiring `O(size(A, 2), nnz(A))`-time for matrices and `O(nnz(A))`-time for vectors -and no space beyond that passed in. If `trim` is `true`, this method trims `rowvals(A)` or `nonzeroinds(A)` and -`nonzeros(A)` to length `nnz(A)` after dropping elements. +and no space beyond that passed in. # Examples ```jldoctest @@ -1328,50 +1327,40 @@ function fkeep!(A::AbstractSparseMatrixCSC, f, trim::Bool = true) Acolptr[Aj+1] = Awritepos end - # Trim A's storage if necessary and desired - if trim - Annz = Acolptr[end] - 1 - if length(Arowval) != Annz - resize!(Arowval, Annz) - end - if length(Anzval) != Annz - resize!(Anzval, Annz) - end - end + # Trim A's storage if necessary + Annz = Acolptr[end] - 1 + resize!(Arowval, Annz) + resize!(Anzval, Annz) - A + return A end -tril!(A::AbstractSparseMatrixCSC, k::Integer = 0, trim::Bool = true) = - fkeep!(A, (i, j, x) -> i + k >= j, trim) -triu!(A::AbstractSparseMatrixCSC, k::Integer = 0, trim::Bool = true) = - fkeep!(A, (i, j, x) -> j >= i + k, trim) +tril!(A::AbstractSparseMatrixCSC, k::Integer = 0) = + fkeep!(A, (i, j, x) -> i + k >= j) +triu!(A::AbstractSparseMatrixCSC, k::Integer = 0) = + fkeep!(A, (i, j, x) -> j >= i + k) """ - droptol!(A::AbstractSparseMatrixCSC, tol; trim::Bool = true) + droptol!(A::AbstractSparseMatrixCSC, tol) -Removes stored values from `A` whose absolute value is less than or equal to `tol`, -optionally trimming resulting excess space from `rowvals(A)` and `nonzeros(A)` when `trim` -is `true`. +Removes stored values from `A` whose absolute value is less than or equal to `tol`. """ -droptol!(A::AbstractSparseMatrixCSC, tol; trim::Bool = true) = - fkeep!(A, (i, j, x) -> abs(x) > tol, trim) +droptol!(A::AbstractSparseMatrixCSC, tol) = + fkeep!(A, (i, j, x) -> abs(x) > tol) """ - dropzeros!(A::AbstractSparseMatrixCSC; trim::Bool = true) + dropzeros!(A::AbstractSparseMatrixCSC;) -Removes stored numerical zeros from `A`, optionally trimming resulting excess space from -`rowvals(A)` and `nonzeros(A)` when `trim` is `true`. +Removes stored numerical zeros from `A`. For an out-of-place version, see [`dropzeros`](@ref). For algorithmic information, see `fkeep!`. """ -dropzeros!(A::AbstractSparseMatrixCSC; trim::Bool = true) = fkeep!(A, (i, j, x) -> !iszero(x), trim) +dropzeros!(A::AbstractSparseMatrixCSC) = fkeep!(A, (i, j, x) -> !iszero(x)) """ - dropzeros(A::AbstractSparseMatrixCSC; trim::Bool = true) + dropzeros(A::AbstractSparseMatrixCSC;) -Generates a copy of `A` and removes stored numerical zeros from that copy, optionally -trimming excess space from the result's `rowval` and `nzval` arrays when `trim` is `true`. +Generates a copy of `A` and removes stored numerical zeros from that copy. For an in-place version and algorithmic information, see [`dropzeros!`](@ref). @@ -1389,7 +1378,7 @@ julia> dropzeros(A) [3, 3] = 1.0 ``` """ -dropzeros(A::AbstractSparseMatrixCSC; trim::Bool = true) = dropzeros!(copy(A), trim = trim) +dropzeros(A::AbstractSparseMatrixCSC) = dropzeros!(copy(A)) ## Find methods diff --git a/stdlib/SparseArrays/src/sparsevector.jl b/stdlib/SparseArrays/src/sparsevector.jl index ef8b2001f519c..398cc550c30e5 100644 --- a/stdlib/SparseArrays/src/sparsevector.jl +++ b/stdlib/SparseArrays/src/sparsevector.jl @@ -1920,7 +1920,7 @@ function sort(x::SparseVector{Tv,Ti}; kws...) where {Tv,Ti} SparseVector(n,newnzind,newnzvals) end -function fkeep!(x::SparseVector, f, trim::Bool = true) +function fkeep!(x::SparseVector, f) n = length(x::SparseVector) nzind = nonzeroinds(x) nzval = nonzeros(x) @@ -1939,43 +1939,35 @@ function fkeep!(x::SparseVector, f, trim::Bool = true) end end - # Trim x's storage if necessary and desired - if trim - x_nnz = x_writepos - 1 - if length(nzind) != x_nnz - resize!(nzval, x_nnz) - resize!(nzind, x_nnz) - end - end + # Trim x's storage if necessary + x_nnz = x_writepos - 1 + resize!(nzval, x_nnz) + resize!(nzind, x_nnz) - x + return x end """ - droptol!(x::SparseVector, tol; trim::Bool = true) + droptol!(x::SparseVector, tol) -Removes stored values from `x` whose absolute value is less than or equal to `tol`, -optionally trimming resulting excess space from `nonzeroinds(x)` and `nonzeros(x)` when `trim` -is `true`. +Removes stored values from `x` whose absolute value is less than or equal to `tol`. """ -droptol!(x::SparseVector, tol; trim::Bool = true) = fkeep!(x, (i, x) -> abs(x) > tol, trim) +droptol!(x::SparseVector, tol) = fkeep!(x, (i, x) -> abs(x) > tol) """ - dropzeros!(x::SparseVector; trim::Bool = true) + dropzeros!(x::SparseVector) -Removes stored numerical zeros from `x`, optionally trimming resulting excess space from -`nonzeroinds(x)` and `nonzeros(x)` when `trim` is `true`. +Removes stored numerical zeros from `x`. For an out-of-place version, see [`dropzeros`](@ref). For algorithmic information, see `fkeep!`. """ -dropzeros!(x::SparseVector; trim::Bool = true) = fkeep!(x, (i, x) -> !iszero(x), trim) +dropzeros!(x::SparseVector) = fkeep!(x, (i, x) -> !iszero(x)) """ - dropzeros(x::SparseVector; trim::Bool = true) + dropzeros(x::SparseVector) -Generates a copy of `x` and removes numerical zeros from that copy, optionally trimming -excess space from the result's `nzind` and `nzval` arrays when `trim` is `true`. +Generates a copy of `x` and removes numerical zeros from that copy. For an in-place version and algorithmic information, see [`dropzeros!`](@ref). @@ -1993,7 +1985,7 @@ julia> dropzeros(A) [3] = 1.0 ``` """ -dropzeros(x::SparseVector; trim::Bool = true) = dropzeros!(copy(x), trim = trim) +dropzeros(x::SparseVector) = dropzeros!(copy(x)) function copy!(dst::SparseVector, src::SparseVector) length(dst::SparseVector) == length(src::SparseVector) || throw(ArgumentError("Sparse vectors should have the same length for copy!")) diff --git a/stdlib/SparseArrays/test/sparse.jl b/stdlib/SparseArrays/test/sparse.jl index 3b00f074137f3..c717e034a75e6 100644 --- a/stdlib/SparseArrays/test/sparse.jl +++ b/stdlib/SparseArrays/test/sparse.jl @@ -1679,15 +1679,11 @@ end for Awithzeros in (Aposzeros, Anegzeros, Abothsigns) # Basic functionality / dropzeros! @test dropzeros!(copy(Awithzeros)) == A - @test dropzeros!(copy(Awithzeros), trim = false) == A # Basic functionality / dropzeros @test dropzeros(Awithzeros) == A - @test dropzeros(Awithzeros, trim = false) == A # Check trimming works as expected @test length(nonzeros(dropzeros!(copy(Awithzeros)))) == length(nonzeros(A)) @test length(rowvals(dropzeros!(copy(Awithzeros)))) == length(rowvals(A)) - @test length(nonzeros(dropzeros!(copy(Awithzeros), trim = false))) == length(nonzeros(Awithzeros)) - @test length(rowvals(dropzeros!(copy(Awithzeros), trim = false))) == length(rowvals(Awithzeros)) end end # original lone dropzeros test diff --git a/stdlib/SparseArrays/test/sparsevector.jl b/stdlib/SparseArrays/test/sparsevector.jl index 9f7e338e57174..65201d3d4dd0f 100644 --- a/stdlib/SparseArrays/test/sparsevector.jl +++ b/stdlib/SparseArrays/test/sparsevector.jl @@ -1168,15 +1168,11 @@ end for vwithzeros in (vposzeros, vnegzeros, vbothsigns) # Basic functionality / dropzeros! @test dropzeros!(copy(vwithzeros)) == v - @test dropzeros!(copy(vwithzeros), trim = false) == v # Basic functionality / dropzeros @test dropzeros(vwithzeros) == v - @test dropzeros(vwithzeros, trim = false) == v # Check trimming works as expected @test length(nonzeros(dropzeros!(copy(vwithzeros)))) == length(nonzeros(v)) @test length(nonzeroinds(dropzeros!(copy(vwithzeros)))) == length(nonzeroinds(v)) - @test length(nonzeros(dropzeros!(copy(vwithzeros), trim = false))) == length(nonzeros(vwithzeros)) - @test length(nonzeroinds(dropzeros!(copy(vwithzeros), trim = false))) == length(nonzeroinds(vwithzeros)) end end