-
Notifications
You must be signed in to change notification settings - Fork 92
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
[Port] A proposal of an all_simple_paths function implementation #20
Conversation
Add a function that finds all simple paths between two nodes in a graph.
To improve memory effeciency, make the stack store only parent node and index.
Codecov Report
@@ Coverage Diff @@
## master #20 +/- ##
=======================================
Coverage 99.45% 99.46%
=======================================
Files 106 107 +1
Lines 5554 5609 +55
=======================================
+ Hits 5524 5579 +55
Misses 30 30 |
What about making the default cutoff smaller? typemax(T) is just completely impractical right? Even in 32 bit int that is ~10^10. What about min(typemax(T), nv(g)^3) or something? Also, I think we should throw an exception if you hit the cutoff. That way people know that they should either increase the cutoff and try again or give up on enumerating all the paths in the graph. Silently giving an incomplete enumeration isn't the right answer. |
@etienneINSA, @matbesancon, Any thoughts on raising an exception if we don't yield all the simple paths? I've never needed all the simple paths, so I don't really know the application context, but I would expect that if I got a partial result, it would come with some return code I could check, or exception that I needed to catch. |
A helper function that updates iterator state. | ||
For internal use only. |
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.
A helper function that updates iterator state. | |
For internal use only. | |
A helper function that updates the simple path iterator state. |
Internal use is already implied by the underscore
When does this happen? I didn't see this path in the code? |
This is not implemented yet, just a thought on if we need to implement it.
Agreed. I don't have a strong opinion on how to do that, but it is also possible to just return a flag. |
Related to #106 |
What is the current status of this PR? I.e. who's turn is is to review/write code? |
I was not aware we had defined turns, I thought it was more of a "whoever has time does their bit". As far as I'm concerned, I need to hand in my thesis manuscript in a month so i'm out of the equation for now |
No of course there is no formal process, I was just trying to understand where we are :) Good look with your manuscript,, I will try to avoid pinging you any further then for now. |
On the contrary, do ping me when you need a quick opinion, it's the only way you'll get my attention ^^ But I won't be able to contribute lengthy reviews or code until the summer has elapsed |
@samurai-kant We need this |
struct SimplePathIterator{T <: Integer} | ||
g::AbstractGraph | ||
source::T # Starting node | ||
targets::Set{T} # Target nodes |
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.
This doesn't have to be a Set
, right? Maybe just make it targets::T
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.
This is designed to support multi-targets, and I wanted to be consistent with the networkx implementation.
Users can use the single target API :
all_simple_paths(g::AbstractGraph, source::T, target::T; cutoff::T=typemax(T)) where T <: Integer
By using outneighbors, does this work for directed and undirected graphs? |
Hi, I am the author of the original implementation. I will answer some questions.
Yes. This support both. See the example below. println("undir:", collect(all_simple_paths(cycle_graph(4), 1, [3])))
println("dir:", collect(all_simple_paths(cycle_digraph(4), 1, [3])))
> undir[[1, 2, 3], [1, 4, 3]]
> dir[[1, 2, 3]]
The resulting paths do not have any loop. So the
Would you clarify the meaning of "incomplete enumeration" in your comment? g = complete_graph(5)
path_iter = Iterators.Stateful(all_simple_paths(g, 1, 2))
println("first 3:", collect(Iterators.take(path_iter, 3)))
println("rests:", collect(path_iter))
> first 3:Any[[1, 2], [1, 3, 2], [1, 3, 4, 2]]
> rests:Any[[1, 3, 4, 5, 2], [1, 3, 5, 2], [1, 3, 5, 4, 2], [1, 4, 2], [1, 4, 3, 2], [1, 4, 3, 5, 2], [1, 4, 5, 2], [1, 4, 5, 3, 2], [1, 5, 2], [1, 5, 3, 2], [1, 5, 3, 4, 2], [1, 5, 4, 2], [1, 5, 4, 3, 2]] If you want to increase the depth gradually, we need to implement a breadth-first path search. It is possible, but I think the bfs requires much more memory footprint. Please let me know if there is anything I can do to help you move forward with your review. |
using DataStructures | ||
|
||
""" | ||
all_simple_paths(g, source, targets, cutoff) |
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.
all_simple_paths(g, source, targets, cutoff) | |
all_simple_paths(g, source, targets; cutoff) |
Thank you, and sorry that this got stalled, @i-aki-y . I think this is fine, setting the cutoff to typemax(Int) just means there is no cut-off which is a reasonable default even if unsuitable for large graphs. |
I have no push access, can't proceed here (e.g. resolving the conflicts). |
It's weird, cause as a member of JuliaGraphs you should be able to work on this PR. Maybe @etiennedeg can give you access to his branch? |
- this updates the port of sbromberger/LightGraphs.jl#1540 from JuliaGraphs#20 - has a number of simplifications relative to original implementation - original implementation by @i_aki_y - cutoff now defaults to `nv(g)` Co-authored-by: i_aki_y Co-authored-by: etiennedeg
- this updates the port of sbromberger/LightGraphs.jl#1540 from JuliaGraphs#20 - has a number of simplifications relative to original implementation - original implementation by @i_aki_y - cutoff now defaults to `nv(g)` Co-authored-by: @i_aki_y Co-authored-by: @etiennedeg
- this updates the port of sbromberger/LightGraphs.jl#1540 from JuliaGraphs#20 - has a number of simplifications relative to original implementation - original implementation by @i-aki-y - cutoff now defaults to `nv(g)` Co-authored-by: @i-aki-y Co-authored-by: @etiennedeg
- this updates the port of sbromberger/LightGraphs.jl#1540 from JuliaGraphs#20 - has a number of simplifications relative to original implementation - original implementation by @i-aki-y - cutoff now defaults to `nv(g)` Co-authored-by: @i-aki-y Co-authored-by: Etienne dg <etienne.mace_de_gastines@insa-rouen.fr>
- this updates the port of sbromberger/LightGraphs.jl#1540 from JuliaGraphs#20 - has a number of simplifications relative to original implementation - original implementation by @i-aki-y - cutoff now defaults to `nv(g)` Co-authored-by: akiyuki ishikawa <aki.y.ishikawa@gmail.com> Co-authored-by: Etienne dg <etienne.mace_de_gastines@insa-rouen.fr>
* `all_simple_paths`: update PR #20 - this updates the port of sbromberger/LightGraphs.jl#1540 from #20 - has a number of simplifications relative to original implementation - original implementation by @i-aki-y - cutoff now defaults to `nv(g)` Co-authored-by: akiyuki ishikawa <aki.y.ishikawa@gmail.com> Co-authored-by: Etienne dg <etienne.mace_de_gastines@insa-rouen.fr> * fixes to tests & doctests * improve docstring * run JuliaFormatter - `format(Graphs, overwrite=true)` * bump to v1.9.1 * fix docs * address code-review * fix formatting * special-case `u in vs` input: include 0-length path `[u]` in iterates * updates after code review * Update src/traversals/all_simple_paths.jl Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * Update src/traversals/all_simple_paths.jl Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * Update src/traversals/all_simple_paths.jl Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * more updates from code-review * format --------- Co-authored-by: akiyuki ishikawa <aki.y.ishikawa@gmail.com> Co-authored-by: Etienne dg <etienne.mace_de_gastines@insa-rouen.fr> Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com>
Superseded by #353 |
this is a port of #1540