-
Notifications
You must be signed in to change notification settings - Fork 198
Improve all simple paths multi-target performance #1520
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
Conversation
Pull Request Test Coverage Report for Build 19041961653Details
💛 - Coveralls |
|
Here is a temporary benchmark's output, where you can see an improvement when comparing The benchmark code for reference: |
IvanIsCoding
left a comment
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.
Firstly, thanks for the contribution. It seems you ported #577 to petgraph/petgraph#865 and made the optimizations.
It just needs some minor changes, but this should be good to go.
releasenotes/notes/replace-api-for-all-simple-paths-0c44a6c18dbdaaf4.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/replace-api-for-all-simple-paths-0c44a6c18dbdaaf4.yaml
Outdated
Show resolved
Hide resolved
Thanks for the comments and they are now all resolved! |
This PR continues the work from #1488.
Now that multiple-target support for
all_simple_pathshas been added topetgraph(see all_simple_path_multi and petgraph/petgraph#865), this PR simply update the underlying implementation fromrustworkx_core::connectivity::all_simple_paths_multiple_targetstopetgraph::algo::all_simple_paths_multi.This avoids the need to flatten
DictMapinto aVecand results in better performance and cleaner code.