From f6dc1a684ded08893872776a8efef5e7f744b1ce Mon Sep 17 00:00:00 2001 From: Alexis Montoison Date: Fri, 14 Feb 2025 16:14:48 -0600 Subject: [PATCH 1/9] Implement our own structure Forest --- Project.toml | 2 -- src/SparseMatrixColorings.jl | 2 +- src/coloring.jl | 32 +++++++++------------- src/forest.jl | 52 ++++++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 23 deletions(-) create mode 100644 src/forest.jl diff --git a/Project.toml b/Project.toml index 157b96e6..c31b0402 100644 --- a/Project.toml +++ b/Project.toml @@ -5,7 +5,6 @@ version = "0.4.15" [deps] ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b" -DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8" DocStringExtensions = "ffbed154-4ef7-542d-bbb7-c09d3a79fcae" LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" @@ -23,7 +22,6 @@ SparseMatrixColoringsColorsExt = "Colors" ADTypes = "1.2.1" CliqueTrees = "0.5.2" Colors = "0.12.11, 0.13" -DataStructures = "0.18" DocStringExtensions = "0.8,0.9" LinearAlgebra = "<0.0.1, 1" Random = "<0.0.1, 1" diff --git a/src/SparseMatrixColorings.jl b/src/SparseMatrixColorings.jl index 35b9857c..45cb7663 100644 --- a/src/SparseMatrixColorings.jl +++ b/src/SparseMatrixColorings.jl @@ -11,7 +11,6 @@ module SparseMatrixColorings using ADTypes: ADTypes using Base.Iterators: Iterators -using DataStructures: DisjointSets, find_root!, root_union!, num_groups using DocStringExtensions: README, EXPORTS, SIGNATURES, TYPEDEF, TYPEDFIELDS using LinearAlgebra: Adjoint, @@ -43,6 +42,7 @@ using SparseArrays: spzeros include("graph.jl") +include("forest.jl") include("order.jl") include("coloring.jl") include("result.jl") diff --git a/src/coloring.jl b/src/coloring.jl index 3246a299..0b8f56fb 100644 --- a/src/coloring.jl +++ b/src/coloring.jl @@ -302,11 +302,7 @@ function acyclic_coloring(g::AdjacencyGraph, order::AbstractOrder; postprocessin forbidden_colors = zeros(Int, nv) first_neighbor = fill((0, 0), nv) # at first no neighbors have been encountered first_visit_to_tree = fill((0, 0), ne) - forest = DisjointSets{Tuple{Int,Int}}() - sizehint!(forest.intmap, ne) - sizehint!(forest.revmap, ne) - sizehint!(forest.internal.parents, ne) - sizehint!(forest.internal.ranks, ne) + forest = Forest{Int}(ne) vertices_in_order = vertices(g, order) for v in vertices_in_order @@ -347,7 +343,7 @@ function acyclic_coloring(g::AdjacencyGraph, order::AbstractOrder; postprocessin end # compress forest - for edge in forest.revmap + for edge in keys(forest.intmap) find_root!(forest, edge) end tree_set = TreeSet(forest, nb_vertices(g)) @@ -367,11 +363,10 @@ function _prevent_cycle!( # modified first_visit_to_tree::AbstractVector{<:Tuple}, forbidden_colors::AbstractVector{<:Integer}, - forest::DisjointSets{<:Tuple{Int,Int}}, + forest::Forest{<:Integer}, ) wx = _sort(w, x) - root = find_root!(forest, wx) # edge wx belongs to the 2-colored tree T represented by edge "root" - id = forest.intmap[root] # ID of the representative edge "root" of a two-colored tree T. + id = find_root!(forest, wx) # The edge wx belongs to the 2-colored tree T, represented by an edge with an integer ID (p, q) = first_visit_to_tree[id] if p != v # T is being visited from vertex v for the first time vw = _sort(v, w) @@ -389,7 +384,7 @@ function _grow_star!( color::AbstractVector{<:Integer}, # modified first_neighbor::AbstractVector{<:Tuple}, - forest::DisjointSets{Tuple{Int,Int}}, + forest::Forest{<:Integer}, ) vw = _sort(v, w) push!(forest, vw) # Create a new tree T_{vw} consisting only of edge vw @@ -412,7 +407,7 @@ function _merge_trees!( w::Integer, x::Integer, # modified - forest::DisjointSets{Tuple{Int,Int}}, + forest::Forest{<:Integer}, ) vw = _sort(v, w) wx = _sort(w, x) @@ -438,12 +433,11 @@ struct TreeSet is_star::Vector{Bool} end -function TreeSet(forest::DisjointSets{Tuple{Int,Int}}, nvertices::Int) - # forest is a structure DisjointSets from DataStructures.jl +function TreeSet(forest::Forest{Int}, nvertices::Int) + # Forest is a structure defined in forest.jl # - forest.intmap: a dictionary that maps an edge (i, j) to an integer k - # - forest.revmap: a dictionary that does the reverse of intmap, mapping an integer k to an edge (i, j) - # - forest.internal.ngroups: the number of trees in the forest - ntrees = forest.internal.ngroups + # - forest.ntrees: the number of trees in the forest + ntrees = forest.ntrees # dictionary that maps a tree's root to the index of the tree roots = Dict{Int,Int}() @@ -454,11 +448,9 @@ function TreeSet(forest::DisjointSets{Tuple{Int,Int}}, nvertices::Int) # counter of the number of roots found k = 0 - for edge in forest.revmap + for edge in keys(forest.intmap) i, j = edge - # forest has already been compressed so this doesn't change its state - root_edge = find_root!(forest, edge) - root = forest.intmap[root_edge] + root = find_root!(forest, edge) # Update roots if !haskey(roots, root) diff --git a/src/forest.jl b/src/forest.jl new file mode 100644 index 00000000..bdbf4940 --- /dev/null +++ b/src/forest.jl @@ -0,0 +1,52 @@ +mutable struct Forest{T<:Integer} + counter::T + intmap::Dict{Tuple{T,T},T} + parents::Vector{T} + ranks::Vector{T} + ntrees::T +end + +function Forest{T}(n::Integer) where {T<:Integer} + counter = zero(T) + intmap = Dict{Tuple{T,T},T}() + sizehint!(intmap, n) + parents = collect(Base.OneTo(T(n))) + ranks = zeros(T, T(n)) + ntrees = T(n) + return Forest{T}(counter, intmap, parents, ranks, ntrees) +end + +function Base.push!(forest::Forest{T}, edge::Tuple{T,T}) where {T<:Integer} + forest.counter += 1 + forest.intmap[edge] = forest.counter + forest.ntrees += one(T) + return edge +end + +function _find_root!(parents::Vector{T}, index_edge::T) where {T<:Integer} + @inbounds p = parents[index_edge] + @inbounds if parents[p] != p + parents[index_edge] = p = _find_root!(parents, p) + end + return p +end + +function find_root!(forest::Forest{T}, edge::Tuple{T,T}) where {T<:Integer} + return _find_root!(forest.parents, forest.intmap[edge]) +end + +function root_union!(forest::Forest{T}, index_edge1::T, index_edge2::T) where {T<:Integer} + parents = forest.parents + rks = forest.ranks + @inbounds rank1 = rks[index_edge1] + @inbounds rank2 = rks[index_edge2] + + if rank1 < rank2 + index_edge1, index_edge2 = index_edge2, index_edge1 + elseif rank1 == rank2 + rks[index_edge1] += one(T) + end + @inbounds parents[index_edge2] = index_edge1 + forest.ntrees -= one(T) + return nothing +end From 9c292667a58cf406bfadd58a089dbfbe65e206bd Mon Sep 17 00:00:00 2001 From: Alexis Montoison <35051714+amontoison@users.noreply.github.com> Date: Fri, 14 Feb 2025 17:55:11 -0600 Subject: [PATCH 2/9] Update src/forest.jl --- src/forest.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/forest.jl b/src/forest.jl index bdbf4940..6a8dfd70 100644 --- a/src/forest.jl +++ b/src/forest.jl @@ -12,7 +12,7 @@ function Forest{T}(n::Integer) where {T<:Integer} sizehint!(intmap, n) parents = collect(Base.OneTo(T(n))) ranks = zeros(T, T(n)) - ntrees = T(n) + ntrees = zero(T) return Forest{T}(counter, intmap, parents, ranks, ntrees) end From d964e1159f395e0db6a3535ac38764c973129797 Mon Sep 17 00:00:00 2001 From: Alexis Montoison Date: Sat, 15 Feb 2025 11:44:26 -0600 Subject: [PATCH 3/9] Don't compress the forest before we create a TreeSet --- src/coloring.jl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coloring.jl b/src/coloring.jl index 0b8f56fb..7d8bc1a0 100644 --- a/src/coloring.jl +++ b/src/coloring.jl @@ -342,10 +342,6 @@ function acyclic_coloring(g::AdjacencyGraph, order::AbstractOrder; postprocessin end end - # compress forest - for edge in keys(forest.intmap) - find_root!(forest, edge) - end tree_set = TreeSet(forest, nb_vertices(g)) if postprocessing # Reuse the vector forbidden_colors to compute offsets during post-processing From d622167715b2241027866062b6d3b2996f867a43 Mon Sep 17 00:00:00 2001 From: Alexis Montoison <35051714+amontoison@users.noreply.github.com> Date: Mon, 17 Feb 2025 14:10:12 -0600 Subject: [PATCH 4/9] Apply suggestions from code review Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> --- src/forest.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/forest.jl b/src/forest.jl index 6a8dfd70..eb00cf83 100644 --- a/src/forest.jl +++ b/src/forest.jl @@ -20,12 +20,12 @@ function Base.push!(forest::Forest{T}, edge::Tuple{T,T}) where {T<:Integer} forest.counter += 1 forest.intmap[edge] = forest.counter forest.ntrees += one(T) - return edge + return forest end function _find_root!(parents::Vector{T}, index_edge::T) where {T<:Integer} - @inbounds p = parents[index_edge] - @inbounds if parents[p] != p + p = parents[index_edge] + if parents[p] != p parents[index_edge] = p = _find_root!(parents, p) end return p @@ -38,15 +38,15 @@ end function root_union!(forest::Forest{T}, index_edge1::T, index_edge2::T) where {T<:Integer} parents = forest.parents rks = forest.ranks - @inbounds rank1 = rks[index_edge1] - @inbounds rank2 = rks[index_edge2] + rank1 = rks[index_edge1] + rank2 = rks[index_edge2] if rank1 < rank2 index_edge1, index_edge2 = index_edge2, index_edge1 elseif rank1 == rank2 rks[index_edge1] += one(T) end - @inbounds parents[index_edge2] = index_edge1 + parents[index_edge2] = index_edge1 forest.ntrees -= one(T) return nothing end From fedd619090009457cc2a4b4cb2403bfc4355e4e9 Mon Sep 17 00:00:00 2001 From: Alexis Montoison Date: Mon, 17 Feb 2025 23:40:38 -0600 Subject: [PATCH 5/9] Add a docstring for the structure Forest --- docs/src/dev.md | 1 + src/forest.jl | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/docs/src/dev.md b/docs/src/dev.md index 608d5650..9fff4e65 100644 --- a/docs/src/dev.md +++ b/docs/src/dev.md @@ -27,6 +27,7 @@ SparseMatrixColorings.symmetric_coefficient SparseMatrixColorings.star_coloring SparseMatrixColorings.acyclic_coloring SparseMatrixColorings.group_by_color +SparseMatrixColorings.Forest SparseMatrixColorings.StarSet SparseMatrixColorings.TreeSet ``` diff --git a/src/forest.jl b/src/forest.jl index eb00cf83..5b0eed23 100644 --- a/src/forest.jl +++ b/src/forest.jl @@ -1,8 +1,23 @@ +## Forest + +""" +$TYPEDEF + +Structure that provides fast union-find operations for constructing a forest during acyclic coloring and bicoloring. + +# Fields +""" +$TYPEDFIELDS mutable struct Forest{T<:Integer} + "current number of edges added to the forest" counter::T + "dictionary mapping each edge represented as a tuple of vertices to its unique integer index" intmap::Dict{Tuple{T,T},T} + "vector storing the index of a parent in the tree for each edge, used in union-find operations" parents::Vector{T} + "vector approximating the depth of each tree to optimize path compression" ranks::Vector{T} + "current number of distinct trees in the forest" ntrees::T end From 6000de844cc3a71dbc218e010f0338cce6752b07 Mon Sep 17 00:00:00 2001 From: Alexis Montoison Date: Mon, 17 Feb 2025 23:45:35 -0600 Subject: [PATCH 6/9] Fix the docstring of Forest --- src/forest.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/forest.jl b/src/forest.jl index 5b0eed23..1e9eb075 100644 --- a/src/forest.jl +++ b/src/forest.jl @@ -6,6 +6,8 @@ $TYPEDEF Structure that provides fast union-find operations for constructing a forest during acyclic coloring and bicoloring. # Fields + +$TYPEDFIELDS """ $TYPEDFIELDS mutable struct Forest{T<:Integer} From 3d52bc64cedf3ec55d646dcd0285b83dc3c6b717 Mon Sep 17 00:00:00 2001 From: Alexis Montoison <35051714+amontoison@users.noreply.github.com> Date: Tue, 18 Feb 2025 21:07:07 -0600 Subject: [PATCH 7/9] Update src/forest.jl --- src/forest.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/forest.jl b/src/forest.jl index 1e9eb075..2fbd95d1 100644 --- a/src/forest.jl +++ b/src/forest.jl @@ -9,7 +9,6 @@ Structure that provides fast union-find operations for constructing a forest dur $TYPEDFIELDS """ -$TYPEDFIELDS mutable struct Forest{T<:Integer} "current number of edges added to the forest" counter::T From 3e6d4494ed62b2afb1ac2090f1fc7dd43792e80f Mon Sep 17 00:00:00 2001 From: Alexis Montoison Date: Mon, 24 Mar 2025 15:27:33 -0500 Subject: [PATCH 8/9] Add unit tests for Forest --- src/coloring.jl | 16 +++++------ src/forest.jl | 22 +++++++-------- test/forest.jl | 73 ++++++++++++++++++++++++++++++++++++++++++++++++ test/runtests.jl | 3 ++ 4 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 test/forest.jl diff --git a/src/coloring.jl b/src/coloring.jl index 7d8bc1a0..c1d303f3 100644 --- a/src/coloring.jl +++ b/src/coloring.jl @@ -432,15 +432,15 @@ end function TreeSet(forest::Forest{Int}, nvertices::Int) # Forest is a structure defined in forest.jl # - forest.intmap: a dictionary that maps an edge (i, j) to an integer k - # - forest.ntrees: the number of trees in the forest - ntrees = forest.ntrees + # - forest.num_trees: the number of trees in the forest + nt = forest.num_trees # dictionary that maps a tree's root to the index of the tree roots = Dict{Int,Int}() - sizehint!(roots, ntrees) + sizehint!(roots, nt) # vector of dictionaries where each dictionary stores the neighbors of each vertex in a tree - trees = [Dict{Int,Vector{Int}}() for i in 1:ntrees] + trees = [Dict{Int,Vector{Int}}() for i in 1:nt] # counter of the number of roots found k = 0 @@ -476,11 +476,11 @@ function TreeSet(forest::Forest{Int}, nvertices::Int) degrees = Vector{Int}(undef, nvertices) # reverse breadth first (BFS) traversal order for each tree in the forest - reverse_bfs_orders = [Tuple{Int,Int}[] for i in 1:ntrees] + reverse_bfs_orders = [Tuple{Int,Int}[] for i in 1:nt] # nvmax is the number of vertices of the biggest tree in the forest nvmax = 0 - for k in 1:ntrees + for k in 1:nt nb_vertices_tree = length(trees[k]) nvmax = max(nvmax, nb_vertices_tree) end @@ -490,9 +490,9 @@ function TreeSet(forest::Forest{Int}, nvertices::Int) # Specify if each tree in the forest is a star, # meaning that one vertex is directly connected to all other vertices in the tree - is_star = Vector{Bool}(undef, ntrees) + is_star = Vector{Bool}(undef, nt) - for k in 1:ntrees + for k in 1:nt tree = trees[k] # Boolean indicating whether the current tree is a star (a single central vertex connected to all others) diff --git a/src/forest.jl b/src/forest.jl index 2fbd95d1..71e8bb00 100644 --- a/src/forest.jl +++ b/src/forest.jl @@ -10,32 +10,32 @@ Structure that provides fast union-find operations for constructing a forest dur $TYPEDFIELDS """ mutable struct Forest{T<:Integer} - "current number of edges added to the forest" - counter::T + "current number of edges in the forest" + num_edges::T + "current number of distinct trees in the forest" + num_trees::T "dictionary mapping each edge represented as a tuple of vertices to its unique integer index" intmap::Dict{Tuple{T,T},T} "vector storing the index of a parent in the tree for each edge, used in union-find operations" parents::Vector{T} "vector approximating the depth of each tree to optimize path compression" ranks::Vector{T} - "current number of distinct trees in the forest" - ntrees::T end function Forest{T}(n::Integer) where {T<:Integer} - counter = zero(T) + num_edges = zero(T) + num_trees = zero(T) intmap = Dict{Tuple{T,T},T}() sizehint!(intmap, n) parents = collect(Base.OneTo(T(n))) ranks = zeros(T, T(n)) - ntrees = zero(T) - return Forest{T}(counter, intmap, parents, ranks, ntrees) + return Forest{T}(num_edges, num_trees, intmap, parents, ranks) end function Base.push!(forest::Forest{T}, edge::Tuple{T,T}) where {T<:Integer} - forest.counter += 1 - forest.intmap[edge] = forest.counter - forest.ntrees += one(T) + forest.num_edges += 1 + forest.intmap[edge] = forest.num_edges + forest.num_trees += one(T) return forest end @@ -63,6 +63,6 @@ function root_union!(forest::Forest{T}, index_edge1::T, index_edge2::T) where {T rks[index_edge1] += one(T) end parents[index_edge2] = index_edge1 - forest.ntrees -= one(T) + forest.num_trees -= one(T) return nothing end diff --git a/test/forest.jl b/test/forest.jl new file mode 100644 index 00000000..aedd9741 --- /dev/null +++ b/test/forest.jl @@ -0,0 +1,73 @@ +@testset "Constructor Forest" begin + forest = Forest{Int}(5) + + @test forest.num_edges == 0 + @test forest.num_trees == 0 + @test length(forest.intmap) == 0 + @test length(forest.parents) == 5 + @test all(forest.parents .== 1:5) + @test all(forest.ranks .== 0) +end + +@testset "Push edge" begin + forest = Forest{Int}(5) + + push!(forest, (1, 2)) + @test forest.num_edges == 1 + @test forest.num_trees == 1 + @test haskey(forest.intmap, (1, 2)) + @test forest.intmap[(1, 2)] == 1 + @test forest.num_trees == 1 + + push!(forest, (3, 4)) + @test forest.num_edges == 2 + @test forest.num_trees == 2 + @test haskey(forest.intmap, (3, 4)) + @test forest.intmap[(3, 4)] == 2 + @test forest.num_trees == 2 +end + +@testset "Find root" begin + forest = Forest{Int}(5) + push!(forest, (1, 2)) + push!(forest, (3, 4)) + + @test find_root!(forest, (1, 2)) == 1 + @test find_root!(forest, (3, 4)) == 2 +end + +@testset "Root union" begin + forest = Forest{Int}(5) + push!(forest, (1, 2)) + push!(forest, (4, 5)) + push!(forest, (2, 4)) + @test forest.num_trees = 3 + + root1 = find_root!(forest, (1, 2)) + root3 = find_root!(forest, (2, 4)) + @test root1 != root3 + + root_union!(forest, root1, root3) + @test find_root!(forest, (2, 4)) == 1 + @test forest.parents[1] == 1 + @test forest.parents[3] == 1 + @test forest.ranks[1] == 1 + @test forest.ranks[3] == 0 + @test forest.num_trees = 2 + + root1 = find_root!(forest, (1, 2)) + root2 = find_root!(forest, (4, 5)) + @test root1 != root2 + root_union!(forest, root1, root2) + @test find_root!(forest, (4, 5)) == 1 + @test forest.parents[1] == 1 + @test forest.parents[2] == 1 + @test forest.ranks[1] == 1 + @test forest.ranks[2] == 0 + @test forest.num_trees == 1 + + push!(forest, (1, 4)) + @test forest.num_trees == 2 + @test forest.intmap[(1, 4)] == 4 + @test forest.parents[4] == 4 +end diff --git a/test/runtests.jl b/test/runtests.jl index a56cca5d..b37f65da 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -33,6 +33,9 @@ include("utils.jl") @testset "Graph" begin include("graph.jl") end + @testset "Forest" begin + include("forest.jl") + end @testset "Order" begin include("order.jl") end From 67ad288da8d64469eaac5c5be534bc8a15120dad Mon Sep 17 00:00:00 2001 From: Alexis Montoison Date: Mon, 24 Mar 2025 16:00:03 -0500 Subject: [PATCH 9/9] Fix test/forest.jl --- test/forest.jl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/forest.jl b/test/forest.jl index aedd9741..dc5e6bf0 100644 --- a/test/forest.jl +++ b/test/forest.jl @@ -1,3 +1,6 @@ +using SparseMatrixColorings: Forest, find_root!, root_union! +using Test + @testset "Constructor Forest" begin forest = Forest{Int}(5) @@ -41,7 +44,7 @@ end push!(forest, (1, 2)) push!(forest, (4, 5)) push!(forest, (2, 4)) - @test forest.num_trees = 3 + @test forest.num_trees == 3 root1 = find_root!(forest, (1, 2)) root3 = find_root!(forest, (2, 4)) @@ -53,7 +56,7 @@ end @test forest.parents[3] == 1 @test forest.ranks[1] == 1 @test forest.ranks[3] == 0 - @test forest.num_trees = 2 + @test forest.num_trees == 2 root1 = find_root!(forest, (1, 2)) root2 = find_root!(forest, (4, 5))