Use GenericGraph for testing biconnectivity algorithms (#270)

* Add ignore_missing_comparison=True for JET tests

* Use GenericGraph for testing biconnectivity algorithms

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

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)}

src/biconnectivity/bridge.jl

@@ -40,18 +40,26 @@ 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
             if wi < 1 # initialisation for vertex v
                 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]

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)

test/biconnectivity/articulation.jl

@@ -18,22 +18,22 @@
     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
     end
 
     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

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

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

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",

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