rework of weighted graphs API, fixes #6 and Graphs#42

src/SimpleWeightedGraphs.jl

@@ -59,14 +59,11 @@ convert(::Type{SparseMatrixCSC{T, U}}, g::AbstractSimpleWeightedGraph) where T<:
 
 ### INTERFACE
 
-nv(g::AbstractSimpleWeightedGraph{T, U}) where T where U = T(size(g.weights, 1))
+nv(g::AbstractSimpleWeightedGraph{T, U}) where T where U = T(size(weights(g), 1))
 vertices(g::AbstractSimpleWeightedGraph{T, U}) where T where U = one(T):nv(g)
 eltype(x::AbstractSimpleWeightedGraph{T, U}) where T where U = T
 weighttype(x::AbstractSimpleWeightedGraph{T, U}) where T where U = U
 
-has_edge(g::AbstractSimpleWeightedGraph{T, U}, e::AbstractSimpleWeightedEdge) where T where U =
-    g.weights[dst(e), src(e)] != zero(U)
-
 # handles single-argument edge constructors such as pairs and tuples
 has_edge(g::AbstractSimpleWeightedGraph{T, U}, x) where T where U = has_edge(g, edgetype(g)(x))
 add_edge!(g::AbstractSimpleWeightedGraph{T, U}, x) where T where U = add_edge!(g, edgetype(g)(x))
@@ -88,42 +85,14 @@ function rem_edge!(g::AbstractSimpleWeightedGraph{T, U}, u::Integer, v::Integer)
     rem_edge!(g, edgetype(g)(T(u), T(v), one(U)))
 end
 
-@doc_str """
-    rem_vertex!(g::AbstractSimpleWeightedGraph, v)
-
-Remove the vertex `v` from graph `g`. Return false if removal fails
-(e.g., if vertex is not in the graph); true otherwise.
-
-### Implementation Notes
-This operation has to be performed carefully if one keeps external
-data structures indexed by edges or vertices in the graph, since
-internally the removal results in all vertices with indices greater than `v`
-being shifted down one.
-"""
-function rem_vertex!(g::AbstractSimpleWeightedGraph, v::Integer)
-    v in vertices(g) || return false
-    n = nv(g)
-
-    newweights = g.weights[1:nv(g) .!= v, :]
-    newweights = newweights[:, 1:nv(g) .!= v]
-
-    g.weights = newweights
-    return true
-end
-
-function outneighbors(g::AbstractSimpleWeightedGraph, v::Integer)
-    mat = g.weights
-    return view(mat.rowval, mat.colptr[v]:mat.colptr[v+1]-1)
-end
-
-get_weight(g::AbstractSimpleWeightedGraph, u::Integer, v::Integer) = g.weights[v, u]
+get_weight(g::AbstractSimpleWeightedGraph, u::Integer, v::Integer) = weights(g)[v, u]
 
 zero(g::T) where T<:AbstractSimpleWeightedGraph = T()
 
 # TODO: manipulte SparseMatrixCSC directly
 add_vertex!(g::AbstractSimpleWeightedGraph) = add_vertices!(g, 1)
 
-copy(g::T) where T <: AbstractSimpleWeightedGraph =  T(copy(g.weights))
+copy(g::T) where T <: AbstractSimpleWeightedGraph =  T(copy(weights(g)))
 
 
 const SimpleWeightedGraphEdge = SimpleWeightedEdge
@@ -136,6 +105,21 @@ include("persistence.jl")
 const WGraph = SimpleWeightedGraph
 const WDiGraph = SimpleWeightedDiGraph
 
+
+# return the index in nzval of mat[i, j]
+# we assume bounds are already checked
+# see https://github.com/JuliaSparse/SparseArrays.jl/blob/fa547689947fadd6c2f3d09ddfcb5f26536f18c8/src/sparsematrix.jl#L2492 for implementation
+@inbounds function _get_nz_index!(mat::SparseMatrixCSC, i::Integer, j::Integer)
+    # r1 and r2 are start and end of the column
+    r1 = Int(mat.colptr[j])
+    r2 = Int(mat.colptr[j+1]-1)
+    (r1 > r2) && return 0 # column is empty so we have a non structural zero
+    # search if i correspond to a stored value
+    indx = searchsortedfirst(mat.rowval, i, r1, r2, Base.Forward)
+    ((indx > r2) || (mat.rowval[indx] != i)) && return 0
+    return indx
+end
+
 SimpleWeightedDiGraph(g::SimpleWeightedGraph) = SimpleWeightedDiGraph(copy(g.weights))
 function SimpleWeightedDiGraph{T, U}(g::SimpleWeightedGraph) where {T<:Integer, U<:Real}
     return SimpleWeightedDiGraph(SparseMatrixCSC{U, T}(copy(g.weights)))

src/simpleweighteddigraph.jl

@@ -40,6 +40,14 @@ SimpleWeightedDiGraph{T,U}(g::SimpleWeightedDiGraph) where {T <: Integer, U <: R
 
 ne(g::SimpleWeightedDiGraph) = nnz(g.weights)
 
+function has_edge(g::SimpleWeightedDiGraph, u::Integer, v::Integer)
+    (u ∈ vertices(g) && v ∈ vertices(g)) || return false
+    _get_nz_index!(g.weights, v, u) != 0 # faster than Base.isstored
+end
+
+has_edge(g::SimpleWeightedDiGraph, e::AbstractEdge) =
+    has_edge(g, src(e), dst(e))
+
 function SimpleWeightedDiGraph{T,U}(n::Integer = 0) where {T<:Integer, U<:Real}
     weights = spzeros(U, T, T(n), T(n))
     return SimpleWeightedDiGraph{T, U}(weights)
@@ -73,7 +81,7 @@ function SimpleWeightedDiGraph(g::Graphs.AbstractGraph{T}, x::U) where {U <: Rea
 end
 
 # DiGraph(srcs, dsts, weights)
-function SimpleWeightedDiGraph(i::AbstractVector{T}, j::AbstractVector{T}, v::AbstractVector{U}; combine = +) where T<:Integer where U<:Real
+function SimpleWeightedDiGraph(i::AbstractVector{T}, j::AbstractVector{T}, v::AbstractVector{U}; combine = +) where {T<:Integer, U<:Real}
     m = max(maximum(j), maximum(i))
     SimpleWeightedDiGraph{T, U}(sparse(j, i, v, m, m, combine), permute=false)
 end
@@ -85,6 +93,10 @@ edgetype(::SimpleWeightedDiGraph{T, U}) where T<:Integer where U<:Real = SimpleW
 edges(g::SimpleWeightedDiGraph) = (SimpleWeightedEdge(x[2], x[1], x[3]) for x in zip(findnz(g.weights)...))
 weights(g::SimpleWeightedDiGraph) = g.weights'
 
+function outneighbors(g::SimpleWeightedDiGraph, v::Integer)
+    mat = g.weights
+    return view(mat.rowval, mat.colptr[v]:(mat.colptr[v+1]-1))
+end
 inneighbors(g::SimpleWeightedDiGraph, v::Integer) = g.weights[v,:].nzind
 
 # add_edge! will overwrite weights.
@@ -105,13 +117,41 @@ function add_edge!(g::SimpleWeightedDiGraph, e::SimpleWeightedGraphEdge)
     return true
 end
 
-function rem_edge!(g::SimpleWeightedDiGraph, e::SimpleWeightedGraphEdge)
-    has_edge(g, e) || return false
-    U = weighttype(g)
-    @inbounds g.weights[dst(e), src(e)] = zero(U)
+rem_edge!(g::SimpleWeightedDiGraph, e::AbstractEdge) =
+    rem_edge!(g, src(e), dst(e))
+
+function rem_edge!(g::SimpleWeightedDiGraph{T, U}, u::Integer, v::Integer) where {T<:Integer, U<:Real}
+    (u ∈ vertices(g) && v ∈ vertices(g)) || return false
+    w = g.weights
+    indx = _get_nz_index!(w, v, u) # get the index in nzval
+    indx == 0 && return false # the edge does not exist
+    @view(w.colptr[(u+one(u)):end]) .-= T(1) # there is one value less in column u
+    # we remove the stored value
+    deleteat!(w.rowval, indx)
+    deleteat!(w.nzval, indx)
+
     return true
 end
 
+@doc_str """
+    rem_vertex!(g::SimpleWeightedDiGraph, v)
+
+Remove the vertex `v` from graph `g`. Return false if removal fails
+(e.g., if vertex is not in the graph); true otherwise.
+
+### Implementation Notes
+This operation has to be performed carefully if one keeps external
+data structures indexed by edges or vertices in the graph, since
+internally the removal results in all vertices with indices greater than `v`
+being shifted down one.
+"""
+function rem_vertex!(g::SimpleWeightedDiGraph, v::Integer)
+    v in vertices(g) || return false
+    n = nv(g)
+    all_except_v = (1:n) .!= v
+    g.weights = g.weights[all_except_v, all_except_v]
+    return true
+end
 
 copy(g::SimpleWeightedDiGraph) =  SimpleWeightedDiGraph(copy(g.weights'))
 
@@ -124,7 +164,7 @@ is_directed(::Type{<:SimpleWeightedDiGraph}) = true
 
 Equivalent to g[src(e), dst(e)].
 """
-function Base.getindex(g::SimpleWeightedDiGraph{T, U}, e::AbstractEdge, ::Val{:weight}) where {T, U}
+function Base.getindex(g::SimpleWeightedDiGraph, e::AbstractEdge, ::Val{:weight})
     return g.weights[dst(e), src(e)]
 end
 
@@ -133,6 +173,6 @@ end
 
 Return the weight of edge (i, j).
 """
-function Base.getindex(g::SimpleWeightedDiGraph{T, U}, i::Integer, j::Integer, ::Val{:weight}) where {T, U}
+function Base.getindex(g::SimpleWeightedDiGraph, i::Integer, j::Integer, ::Val{:weight})
     return g.weights[j, i]
 end

src/simpleweightedgraph.jl

@@ -20,6 +20,14 @@ end
 
 ne(g::SimpleWeightedGraph) = (nnz(g.weights) + nselfloop(g)) ÷ 2
 
+function has_edge(g::SimpleWeightedGraph, u::Integer, v::Integer)
+    u ∈ vertices(g) && v ∈ vertices(g) || return false
+    _get_nz_index!(g.weights, v, u) != 0 # faster than Base.isstored
+end
+
+has_edge(g::SimpleWeightedGraph, e::AbstractEdge) =
+    has_edge(g, src(e), dst(e))
+
 function nselfloop(g::SimpleWeightedGraph)
     n = 0
     for i in axes(g.weights, 1)
@@ -111,6 +119,11 @@ edgetype(::SimpleWeightedGraph{T, U}) where {T<:Integer, U<:Real} = SimpleWeight
 
 edges(g::SimpleWeightedGraph) = (SimpleWeightedEdge(x[1], x[2], x[3]) for x in zip(findnz(triu(g.weights))...))
 weights(g::SimpleWeightedGraph) = g.weights
+
+function outneighbors(g::SimpleWeightedGraph, v::Integer)
+    mat = g.weights
+    return view(mat.rowval, mat.colptr[v]:mat.colptr[v+1]-1)
+end
 inneighbors(g::SimpleWeightedGraph, x...) = outneighbors(g, x...)
 
 # add_edge! will overwrite weights.
@@ -132,11 +145,44 @@ function add_edge!(g::SimpleWeightedGraph, e::SimpleWeightedGraphEdge)
     return true
 end
 
-function rem_edge!(g::AbstractSimpleWeightedGraph, e::SimpleWeightedGraphEdge)
-    has_edge(g, e) || return false
-    U = weighttype(g)
-    @inbounds g.weights[dst(e), src(e)] = zero(U)
-    @inbounds g.weights[src(e), dst(e)] = zero(U)
+rem_edge!(g::SimpleWeightedGraph, e::AbstractEdge) =
+    rem_edge!(g, src(e), dst(e))
+
+function rem_edge!(g::SimpleWeightedGraph{T, U}, u::Integer, v::Integer) where {T<:Integer, U<:Real}
+    (u ∈ vertices(g) && v ∈ vertices(g)) || return false
+    w = g.weights
+    indx_uv = _get_nz_index!(w, u, v) # get the index in nzval
+    indx_uv == 0 && return false # the edge does not exist
+    @view(w.colptr[(v+one(v)):end]) .-= T(1) # there is one value less in column v
+    # we remove the stored value
+    deleteat!(w.rowval, indx_uv)
+    deleteat!(w.nzval, indx_uv)
+    (u == v) && return true
+    # same for the reverse edge
+    indx_vu = _get_nz_index!(w, v, u)
+    @view(w.colptr[(u+one(u)):end]) .-= T(1)
+    deleteat!(w.rowval, indx_vu)
+    deleteat!(w.nzval, indx_vu)
+    return true
+end
+
+@doc_str """
+    rem_vertex!(g::SimpleWeightedGraph, v)
+
+Remove the vertex `v` from graph `g`. Return false if removal fails
+(e.g., if vertex is not in the graph); true otherwise.
+
+### Implementation Notes
+This operation has to be performed carefully if one keeps external
+data structures indexed by edges or vertices in the graph, since
+internally the removal results in all vertices with indices greater than `v`
+being shifted down one.
+"""
+function rem_vertex!(g::SimpleWeightedGraph, v::Integer)
+    v in vertices(g) || return false
+    n = nv(g)
+    all_except_v = (1:n) .!= v
+    g.weights = g.weights[all_except_v, all_except_v]
     return true
 end
 
@@ -145,10 +191,11 @@ end
 
 is_directed(::Type{<:SimpleWeightedGraph}) = false
 
-function Base.getindex(g::SimpleWeightedGraph{T, U}, e::AbstractEdge, ::Val{:weight}) where {T, U}
+
+function Base.getindex(g::SimpleWeightedGraph, e::AbstractEdge, ::Val{:weight})
     return g.weights[src(e), dst(e)]
 end
 
-function Base.getindex(g::SimpleWeightedGraph{T, U}, i::Integer, j::Integer, ::Val{:weight}) where {T, U}
+function Base.getindex(g::SimpleWeightedGraph, i::Integer, j::Integer, ::Val{:weight})
     return g.weights[i, j]
 end

test/simpleweightedgraph.jl

@@ -74,6 +74,7 @@ using SimpleWeightedGraphs
         @test @inferred(ne(g)) == 3
 
         @test @inferred(rem_edge!(gc, 1, 2)) && @inferred(!has_edge(gc, 1, 2))
+        @test @inferred(inneighbors(gc, 2)) == @inferred(outneighbors(gc, 2)) == @inferred(neighbors(gc,2)) == [3]
         ga = @inferred(copy(g))
         @test @inferred(rem_vertex!(ga, 2)) && ne(ga) == 1
         @test @inferred(!rem_vertex!(ga, 10))
@@ -142,6 +143,7 @@ using SimpleWeightedGraphs
 
         @test @inferred(!rem_edge!(gc, 2, 1))
         @test @inferred(rem_edge!(gc, 1, 2)) && @inferred(!has_edge(gc, 1, 2))
+        @test @inferred(outneighbors(gc, 1)) == @inferred(neighbors(gc, 1)) == Int[]
         ga = @inferred(copy(g))
         @test @inferred(rem_vertex!(ga, 2)) && ne(ga) == 1
         @test @inferred(!rem_vertex!(ga, 10))