-
Notifications
You must be signed in to change notification settings - Fork 160
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
Add dfs-search #456
Add dfs-search #456
Conversation
Widely used graph libraries like Boost Graph Library and graph-tool provide the ability to insert callback functions at specified event points for many common graph search algorithms. petgraph has a similar concept in `petgraph::visit::depth_first_search` function. This commit implements an iterative version of `depth_first_search`, that will be used in a follow-up in the pending PRs Qiskit#444, Qiskit#445. At the same time it exposes this new functionality in Python by letting users subclassing `retworkx.visit.DFSVisitor` and provide their own implementation for the appropriate callback functions. The benefit is less memory consumption since we avoid storing the results but rather let the user take the desired action at specified points. For example, if a user wants to process the nodes in dfs-order, we don't need to create a new list with all the graph nodes in dfs-order but rather the user can process a node on the fly. We can (probably) leverage this approach in other algorithms as an alternative for our inability to provide "real" python iterators.
Pull Request Test Coverage Report for Build 1678805145
💛 - Coveralls |
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 really like this idea. I was thinking about doing something similar for iterating over a dag in topological order (which terra does internally for a bunch of operations). But having this as a starting pattern lets us expose it for that and BFS which will give a lot of flexibility. I haven't reviewed the code in detail yet but the only other thing missing is some test coverage of the new functionality.
…odule together with `dfs_edges` function
…into dfs-search
…the weight of an edge
…port them in rust so we can simplify a bit the namespaces
This commit implements a `breadth_first_search` that provides the ability to insert callback functions at specified event points. Python users should subclass `retworkx.visit.BFSVisitor` and overwrite the appropriate callback functions. Similar to Qiskit#456
* Add bfs-search This commit implements a `breadth_first_search` that provides the ability to insert callback functions at specified event points. Python users should subclass `retworkx.visit.BFSVisitor` and overwrite the appropriate callback functions. Similar to #456 * update docs from code review Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * docs Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * docs + a comment in the code * pseudo-code is not a doc test * fix typo in docs * do not raise if StopSearch exception * more tests * black * add missing example from docs * black again Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* Add bfs-search This commit implements a `breadth_first_search` that provides the ability to insert callback functions at specified event points. Python users should subclass `retworkx.visit.BFSVisitor` and overwrite the appropriate callback functions. Similar to Qiskit#456 * update docs from code review Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * docs Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * docs + a comment in the code * pseudo-code is not a doc test * fix typo in docs * do not raise if StopSearch exception * more tests * black * add missing example from docs * black again Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
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.
Overall this LGTM, thanks for updating it and I'm sorry about the slow review. I think there are 2 minor things inline one around a duplicated macro definition and the other about a missing link to the upstream dfsevent struct in petgraph to show where things came from on it (since it'll have separate documentation). Other than that I think this is ready to go.
Widely used graph libraries like Boost Graph Library and graph-tool
provide the ability to insert callback functions at specified event
points for many common graph search algorithms. petgraph has a similar
concept in
petgraph::visit::depth_first_search
function. This commitimplements an iterative version of
depth_first_search
, that will beused in a follow-up in the pending PRs #444, #445. At the same time it
exposes this new functionality in Python by letting users subclassing
retworkx.visit.DFSVisitor
and provide their own implementation forthe appropriate callback functions.
The benefit is less memory consumption since we avoid storing the results
but rather let the user take the desired action at specified points. For
example, if a user wants to process the nodes in dfs-order, we don't need
to create a new list with all the graph nodes in dfs-order but rather the user
can process a node on the fly. We can (probably) leverage this approach
in other algorithms as an alternative for our inability to provide "real" python
iterators.