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

Matching #400

Merged
merged 8 commits into from
Apr 30, 2021
Merged

Matching #400

merged 8 commits into from
Apr 30, 2021

Conversation

pnevyk
Copy link
Contributor

@pnevyk pnevyk commented Mar 8, 2021

Hi,

I implemented an algorithm for maximum matching. It was also requested in #296.

The implementation uses Gabow's algorithm, which has O(|V|^3) time complexity. The state-of-the-art is O(sqrt(|V|) |E|) (a classical example is Micali-Vazirani). Wikipedia has article about an algorithm which is said to have O(|V|^2 |E|) time.

The issue with the state-of-the-art algorithms is that they are very complex. The algorithm implemented in this PR is much simpler to implement and I consider it a good start. A nice thing about it is that it uses labeling techniques to encode structure and does not require any complex data structures for blossom shrinking and expansion. The ultimate goal (not for this PR) is to implement a O(sqrt(|V|) |E|) algorithm - a good candidate might be this work, which seems to be quite accessible, but I haven't read it yet.

Looking forward to your thoughts and feedback.

@pnevyk pnevyk marked this pull request as ready for review March 8, 2021 14:18
@pnevyk pnevyk changed the title Implement matching Matching Mar 8, 2021
@bluss
Copy link
Member

bluss commented Mar 12, 2021

Hi, at this moment the maintainers don't have so much time unfortunately, so there can be a lot of delay.

@pnevyk
Copy link
Contributor Author

pnevyk commented Mar 12, 2021

That's fine, take your time.

Copy link
Member

@ABorgna ABorgna left a comment

Choose a reason for hiding this comment

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

This is really clean. Thanks!
Can you rebase over master so the failing rustfmt test runs again ?

In addition to the comments, we might want to change the name of maximum_matching to gabow_maximum_matching or similar (as what happens with tarjan_scc and kosaraju_scc) so that future implementations using other algorithms can set different constraints without breaking the API.
@bluss do you have a preference on this?

src/matching.rs Outdated Show resolved Hide resolved
src/matching.rs Outdated Show resolved Hide resolved
@pnevyk
Copy link
Contributor Author

pnevyk commented Apr 28, 2021

Thanks! I will rebase.

Regarding the name change, here is my perspective. I am not sure whether this algorithm should be permanent in petgraph codebase. Its time complexity O(|V|^3) is significantly worse than the best I found O(sqrt(|V|) |E|). But I needed a maximum matching algorithm and chose Gabow for its simplicity as a least effort, and thought that others may benefit from it even though it's not state-of-the-art. I can imagine replacing the algorithm under the hoods in the future.

Nonetheless, thanks to its simplicity, this algorithm may be actually faster than very complicated state-of-the-art on some graphs. In that case, it would make sense to provide it through gabow_maximum_matching.

@bluss
Copy link
Member

bluss commented Apr 29, 2021

@pnevyk's rationale for a more specific name sounds good 🙂

@pnevyk
Copy link
Contributor Author

pnevyk commented Apr 30, 2021

There may be a little trouble however. This paper from 2017 advertises itself as "an accessible matching algorithm with time bound O(sqrt(|V|) |E|)" and by skimming the paper I can agree that it is, compared to what I saw in others. So I planned to give this a try. But the paper is written by Gabow as well.

But I am no expert on matching algorithms, I don't know which one should be included in petgraph.

@ABorgna
Copy link
Member

ABorgna commented Apr 30, 2021

Oh, right. We could check which algorithm are being used by other libraries too, but that one looks nice, simple, and asymptotically optimal.

For now, we can start merging this first implementation. Thanks for the work!

@ABorgna ABorgna merged commit 29fe535 into petgraph:master Apr 30, 2021
teuron pushed a commit to teuron/petgraph that referenced this pull request Oct 9, 2022
* Add initial implementation of matching algorithm

* Fix compilation errors on rustc 1.37.0

* Optimize and enhance the `Matching` structure

* Simplify implementation of iterators in matching module

* Change panics documentation to petgraph format.

* Add support for stable graph

* Fix formatting and matching tests without default features

* Return falsy values instead of panicking in Matching methods
teuron pushed a commit to teuron/petgraph that referenced this pull request Oct 9, 2022
* Add initial implementation of matching algorithm

* Fix compilation errors on rustc 1.37.0

* Optimize and enhance the `Matching` structure

* Simplify implementation of iterators in matching module

* Change panics documentation to petgraph format.

* Add support for stable graph

* Fix formatting and matching tests without default features

* Return falsy values instead of panicking in Matching methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants