-
Notifications
You must be signed in to change notification settings - Fork 6
[WIP] Feature cardinality matching #18
base: master
Are you sure you want to change the base?
Conversation
matching[e.dst] = e.src | ||
matching_len += 1 | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be a good idea, to have this as a separate function? Then it could also be used for finding a greed maximal cardinality matching and be used by other functions. We could pass it in as an optional argument or a keyword argument.
Furthermore, I was thinking, maybe it would be more efficient to loop over the vertices and then over all neighbors of that vertex? I.e something like
for u in vertices(g)
matching[u] == -1 || continue
for v in neighbors[u]
u <= v && continue
matching[u] = v
matching[v] = u
matching_len += 1
end
end
This approach only works, if we assume that vertices(g)
is ordered in increasing order, but it can also be adapted, such that the increasing order is not required.
Also, be careful with self-loops, these should never be in a matching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If neighbors is implemented in a fast manner yes. Thought that looping over the edges might be faster than accessing the neighbors but yours has an earlier break. Maybe I can test it on a larger graph. Good point with self loops!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for SimpleGraph
the edges iterator is implemented using neighbors
anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is now a separate function but still uses edges
for now.
It's kind of late here, so I cannot check now, but does your algorithm handle graphs with odd cycles correctly? Odd cycles are the main reason why maximum matching is non-trivial and why Edmonds came up with his blossom algorithm. |
Yes it's too late here as well. You have a good point though. I came here from bipartite matchings. The algorithm fails because it doesn't allow odd cycles. Works for bipartite graphs because that can't happen there. Learned something again today. Thanks for pointing it out. Either I can check if the graph is bipartite or the PR can be removed as the Blossom algorithm will have the same runtime for bipartite graphs there is no real gain. |
Is this still worked on? If I may, here is a proposition: abstract type AbstractBipartiteGraph <: AbstractGraph end
struct BipartiteGraph <: AbstractBipartiteGraph
...
end
struct CompleteBipartiteGraph <: AbstractBipartiteGraph
...
end You could then dispatch this PR's |
@bilelomrani1 hi! If you get the PR in SpecialGraphs feel free to ping me for review |
Hi @matbesancon, actually I was thinking about filing an issue to get some discussion going around leveraging graph structures for solving graph theoretic problems (including matching). I don't know if special concrete types are the best solution because some graphs may exhibit very special structure (e.g. planar-bipartite) and it does not play nicely with the hierarchical type system. |
Finding a maximum cardinality matching using a simple BFS algorithm to increase the matching weight by 1 every time.
Implements #15
Might be helpful to have more test cases.
Requesting review @matbesancon
Current version only works for bipartite graphs.