diff --git a/src/biconnectivity/articulation.jl b/src/biconnectivity/articulation.jl index 94b4789eb..3156e7a1a 100644 --- a/src/biconnectivity/articulation.jl +++ b/src/biconnectivity/articulation.jl @@ -35,17 +35,25 @@ function articulation end cnt::T = one(T) first_time = true + # TODO the algorithm currently relies on the assumption that + # outneighbors(g, v) is indexable. This assumption might not be true + # in general, so in case that outneighbors does not produce a vector + # we collect these vertices. This might lead to a large number of + # allocations, so we should find a way to handle that case differently, + # or require inneighbors, outneighbors and neighbors to always + # return indexable collections. + while !isempty(s) || first_time first_time = false if wi < 1 pre[v] = cnt cnt += 1 low[v] = pre[v] - v_neighbors = outneighbors(g, v) + v_neighbors = collect_if_not_vector(outneighbors(g, v)) wi = 1 else wi, u, v = pop!(s) - v_neighbors = outneighbors(g, v) + v_neighbors = collect_if_not_vector(outneighbors(g, v)) w = v_neighbors[wi] low[v] = min(low[v], low[w]) if low[w] >= pre[v] && u != v diff --git a/src/biconnectivity/biconnect.jl b/src/biconnectivity/biconnect.jl index 42565c14c..dfc859fc1 100644 --- a/src/biconnectivity/biconnect.jl +++ b/src/biconnectivity/biconnect.jl @@ -11,6 +11,8 @@ mutable struct Biconnections{E<:AbstractEdge} id::Int end +# TODO it might be more reasonable to return the components a s collections of vertices +# instead of edges. @traitfn function Biconnections(g::::(!IsDirected)) n = nv(g) E = Edge{eltype(g)} diff --git a/src/biconnectivity/bridge.jl b/src/biconnectivity/bridge.jl index 07896d445..443f04d27 100644 --- a/src/biconnectivity/bridge.jl +++ b/src/biconnectivity/bridge.jl @@ -40,6 +40,14 @@ function bridges end cnt::T = one(T) # keeps record of the time first_time = true + # TODO the algorithm currently relies on the assumption that + # outneighbors(g, v) is indexable. This assumption might not be true + # in general, so in case that outneighbors does not produce a vector + # we collect these vertices. This might lead to a large number of + # allocations, so we should find a way to handle that case differently, + # or require inneighbors, outneighbors and neighbors to always + # return indexable collections. + # start of DFS while !isempty(s) || first_time first_time = false @@ -47,11 +55,11 @@ function bridges end pre[v] = cnt cnt += 1 low[v] = pre[v] - v_neighbors = outneighbors(g, v) + v_neighbors = collect_if_not_vector(outneighbors(g, v)) wi = 1 else wi, u, v = pop!(s) # the stack states, explained later - v_neighbors = outneighbors(g, v) + v_neighbors = collect_if_not_vector(outneighbors(g, v)) w = v_neighbors[wi] low[v] = min(low[v], low[w]) # condition check for (v, w) being a tree-edge if low[w] > pre[v] diff --git a/src/utils.jl b/src/utils.jl index 770d0d54d..f87638cae 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -297,3 +297,6 @@ function deepcopy_adjlist(adjlist::Vector{Vector{T}}) where {T} return result end + +collect_if_not_vector(xs::AbstractVector) = xs +collect_if_not_vector(xs) = collect(xs) diff --git a/test/biconnectivity/articulation.jl b/test/biconnectivity/articulation.jl index a04135cda..d674c675a 100644 --- a/test/biconnectivity/articulation.jl +++ b/test/biconnectivity/articulation.jl @@ -18,14 +18,14 @@ add_edge!(gint, 7, 10) add_edge!(gint, 7, 12) - for g in testgraphs(gint) + for g in test_generic_graphs(gint) art = @inferred(articulation(g)) ans = [1, 7, 8, 12] @test art == ans end for level in 1:6 btree = Graphs.binary_tree(level) - for tree in [btree, Graph{UInt8}(btree), Graph{Int16}(btree)] + for tree in test_generic_graphs(btree; eltypes=[Int, UInt8, Int16]) artpts = @inferred(articulation(tree)) @test artpts == collect(1:(2^(level - 1) - 1)) end @@ -33,7 +33,7 @@ hint = blockdiag(wheel_graph(5), wheel_graph(5)) add_edge!(hint, 5, 6) - for h in (hint, Graph{UInt8}(hint), Graph{Int16}(hint)) + for h in test_generic_graphs(hint, eltypes=[Int, UInt8, Int16]) @test @inferred(articulation(h)) == [5, 6] end end diff --git a/test/biconnectivity/biconnect.jl b/test/biconnectivity/biconnect.jl index 93e31a42d..6659ae815 100644 --- a/test/biconnectivity/biconnect.jl +++ b/test/biconnectivity/biconnect.jl @@ -23,7 +23,7 @@ [Edge(11, 12)], ] - for g in testgraphs(gint) + for g in test_generic_graphs(gint) bcc = @inferred(biconnected_components(g)) @test bcc == a @test typeof(bcc) === Vector{Vector{Edge{eltype(g)}}} @@ -50,7 +50,7 @@ [Edge(1, 4), Edge(3, 4), Edge(2, 3), Edge(1, 2)], ] - for g in testgraphs(gint) + for g in test_generic_graphs(gint) bcc = @inferred(biconnected_components(g)) @test bcc == a @test typeof(bcc) === Vector{Vector{Edge{eltype(g)}}} @@ -59,6 +59,6 @@ # Non regression test for #13 g = complete_graph(4) a = [[Edge(2, 4), Edge(1, 4), Edge(3, 4), Edge(1, 3), Edge(2, 3), Edge(1, 2)]] - bcc = @inferred(biconnected_components(g)) + bcc = @inferred(biconnected_components(GenericGraph(g))) @test bcc == a end diff --git a/test/biconnectivity/bridge.jl b/test/biconnectivity/bridge.jl index 7526b5fe6..c1d0e0e5b 100644 --- a/test/biconnectivity/bridge.jl +++ b/test/biconnectivity/bridge.jl @@ -20,26 +20,32 @@ add_edge!(gint, 7, 10) add_edge!(gint, 7, 12) - for g in testgraphs(gint) + for g in test_generic_graphs(gint) brd = @inferred(bridges(g)) ans = [Edge(1, 2), Edge(8, 9), Edge(7, 8), Edge(11, 12)] @test brd == ans end for level in 1:6 btree = Graphs.binary_tree(level) - for tree in [btree, Graph{UInt8}(btree), Graph{Int16}(btree)] + for tree in test_generic_graphs(btree; eltypes=[Int, UInt8, Int16]) brd = @inferred(bridges(tree)) - ans = collect(edges(tree)) - @test Set(brd) == Set(ans) + ans = edges(tree) + + # AbstractEdge currently does not implement any comparison operators + # so instead we compare tuples of source and target vertices + @test sort([(src(e), dst(e)) for e in brd]) == sort([(src(e), dst(e)) for e in ans]) end end hint = blockdiag(wheel_graph(5), wheel_graph(5)) add_edge!(hint, 5, 6) - for h in (hint, Graph{UInt8}(hint), Graph{Int16}(hint)) - @test @inferred(bridges(h)) == [Edge(5, 6)] + for h in test_generic_graphs(hint; eltypes=[Int, UInt8, Int16]) + brd = @inferred bridges(h) + @test length(brd) == 1 + @test src(brd[begin]) == 5 + @test dst(brd[begin]) == 6 end - dir = SimpleDiGraph(10, 10; rng=rng) + dir = GenericDiGraph(SimpleDiGraph(10, 10; rng=rng)) @test_throws MethodError bridges(dir) end diff --git a/test/runtests.jl b/test/runtests.jl index d6d64360b..cbb8763bb 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -66,6 +66,22 @@ function testlargegraphs(g) end testlargegraphs(gs...) = vcat((testlargegraphs(g) for g in gs)...) +function test_generic_graphs(g; eltypes=[UInt8, Int16], skip_if_too_large::Bool=false) + SG = is_directed(g) ? SimpleDiGraph : SimpleGraph + GG = is_directed(g) ? GenericDiGraph : GenericGraph + result = GG[] + for T in eltypes + if skip_if_too_large && nv(g) > typemax(T) + continue + end + push!(result, GG(SG{T}(g))) + end + return result +end + +test_large_generic_graphs(g; skip_if_too_large::Bool=false) = test_generic_graphs(g; eltypes=[UInt16, Int32], skip_if_too_large=skip_if_too_large) + + tests = [ "simplegraphs/runtests", "linalg/runtests", diff --git a/test/utils.jl b/test/utils.jl index f9e343908..01277f748 100644 --- a/test/utils.jl +++ b/test/utils.jl @@ -82,3 +82,24 @@ end p = @inferred(Graphs.optimal_contiguous_partition([1, 1, 1, 1], 4)) @test p == [1:1, 2:2, 3:3, 4:4] end + +@testset "collect_if_not_vector" begin + + vectors = [["ab", "cd"], 1:2:9, BitVector([0, 1, 0])] + not_vectors = [Set([1, 2]), (x for x in Int8[3, 4]), "xyz"] + + @testset "identitcal if vector" for v in vectors + @test Graphs.collect_if_not_vector(v) === v + end + + @testset "not identical if not vector" for v in not_vectors + @test Graphs.collect_if_not_vector(v) !== v + end + + @testset "collected if not vector" for v in not_vectors + actual = Graphs.collect_if_not_vector(v) + expected = collect(v) + @test typeof(actual) == typeof(expected) + @test actual == expected + end +end