Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Kruskal traversal iterator #336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/iterators/kruskal.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# The kruskal algorithms are largely copied from Graphs/src/spanningtrees/kruskal.jl

struct KruskalIterator <: EdgeIterator
graph::AbstractGraph
connected_vs::IntDisjointSets
distmx::AbstractMatrix
edge_list

function KruskalIterator(graph, distmx=weights(g); minimize=true)
is_directed(graph) && throw(ArgumentError("Cannot use Kruskal on a directed graph."))
weights = Vector{eltype(distmx)}()
sizehint!(weights, ne(graph))
edge_list = collect(edges(graph))
for e in edge_list
push!(weights, distmx[src(e), dst(e)])
end
e = edge_list[sortperm(weights; rev=!minimize)]
new(graph, IntDisjointSets(nv(graph)), distmx, e)
end
end

Base.length(t::KruskalIterator) = nv(t.graph)-1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that true in case the graph is not connected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are completely correct, but I don't have time to work on this at the moment, instead I would be willing to finish up #163, if you happen to have some review time I think it could be better to redirect it there :-)

(by the way I think also the other PR suffer from the same problem)

Base.eltype(t::KruskalIterator) = edgetype(t.graph)


"""
mutable struct KruskalIteratorState
`KruskalIteratorState` is a struct to hold the current state of iteration which is need for the Base.iterate() function.
"""
mutable struct KruskalIteratorState <: AbstractIteratorState
edge_id::Int
mst_len::Int
end
Comment on lines +30 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the advantage of having a mutable iterator state here? Couldn't we simply return a new state?



function Base.iterate(t::KruskalIterator, state::KruskalIteratorState=KruskalIteratorState(1,1))
while state.mst_len <= (nv(t.graph)-1)
i = state.edge_id
if !in_same_set(t.connected_vs, src(t.edge_list[i]), dst(t.edge_list[i]))
union!(t.connected_vs, src(t.edge_list[i]), dst(t.edge_list[i]))
state.mst_len += 1
return (t.edge_list[i], state)
end
state.edge_id += 1
end
end
Loading