[Port] A proposal of an all_simple_paths function implementation #20

etiennedeg · 2021-10-29T14:30:35Z

this is a port of #1540

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 · 2021-11-07T22:53:30Z

Codecov Report

Merging #20 (acf0d5a) into master (6ab2160) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #20   +/-   ##
=======================================
  Coverage   99.45%   99.46%           
=======================================
  Files         106      107    +1     
  Lines        5554     5609   +55     
=======================================
+ Hits         5524     5579   +55     
  Misses         30       30

jpfairbanks · 2021-11-08T14:46:39Z

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.

jpfairbanks · 2021-11-13T21:27:45Z

@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.

matbesancon · 2021-11-13T22:17:40Z

src/traversals/allsimplepaths.jl

+A helper function that updates iterator state.
+For internal use only.


Suggested change

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

matbesancon · 2021-11-13T22:20:41Z

Any thoughts on raising an exception if we don't yield all the simple paths?

When does this happen? I didn't see this path in the code?

etiennedeg · 2021-11-15T10:42:30Z

Any thoughts on raising an exception if we don't yield all the simple paths?

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.

Silently giving an incomplete enumeration isn't the right answer.

Agreed. I don't have a strong opinion on how to do that, but it is also possible to just return a flag.
Maybe we can implement this as an iterator on simple paths ? (So no cutoff needed)

gdalle · 2022-03-10T07:51:19Z

Related to #106

simonschoelly · 2022-07-24T14:06:49Z

What is the current status of this PR? I.e. who's turn is is to review/write code?

gdalle · 2022-07-24T14:40:42Z

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

simonschoelly · 2022-07-24T14:43:53Z

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.

gdalle · 2022-07-24T14:46:27Z

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

mschauer · 2022-08-17T13:49:43Z

@samurai-kant We need this

mschauer · 2022-08-17T14:03:54Z

src/traversals/allsimplepaths.jl

+struct SimplePathIterator{T <: Integer}
+    g::AbstractGraph
+    source::T  # Starting node
+    targets::Set{T}  # Target nodes


This doesn't have to be a Set, right? Maybe just make it targets::T

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

mschauer · 2022-08-17T14:05:09Z

By using outneighbors, does this work for directed and undirected graphs?

i-aki-y · 2023-04-04T12:41:09Z

Hi, I am the author of the original implementation.
Thank you for being so interested in my PR. I recently found this project!

I will answer some questions.

@mschauer

By using outneighbors, does this work for directed and undirected graphs?

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]]

@jpfairbanks

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?

The resulting paths do not have any loop. So the cutoff upper bound is: nv(g), the paths that visit all nodes. Here, I intended to set no limit for the path length by using typemax. Note that the cutoff does not mean the number of outputs but the maximum length of each individual path, in other words, the max depth of the search path.

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.

Would you clarify the meaning of "incomplete enumeration" in your comment?
If a user set cutoff=c, the user expects the paths whose lengths are less than or equal to c, and the function returns all paths that satisfy the condition. In that sense, the enumeration is complete, I think.
If the user wants to limit the iteration count and restart it afterward, the user can use Iterators.Stateful and Iterators.take.
See:

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.

mschauer · 2024-02-14T15:48:09Z

src/traversals/allsimplepaths.jl

+using DataStructures
+
+"""
+    all_simple_paths(g, source, targets, cutoff)


Suggested change

all_simple_paths(g, source, targets, cutoff)

all_simple_paths(g, source, targets; cutoff)

mschauer · 2024-02-14T15:49:42Z

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.

mschauer · 2024-02-16T23:08:19Z

I have no push access, can't proceed here (e.g. resolving the conflicts).

gdalle · 2024-02-21T12:47:37Z

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

@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

@i-aki-y

- 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

@i-aki-y

- 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 <[email protected]>

@i-aki-y

- 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 <[email protected]> Co-authored-by: Etienne dg <[email protected]>

@i-aki-y

* `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 <[email protected]> Co-authored-by: Etienne dg <[email protected]> * 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 <[email protected]> * Update src/traversals/all_simple_paths.jl Co-authored-by: Guillaume Dalle <[email protected]> * Update src/traversals/all_simple_paths.jl Co-authored-by: Guillaume Dalle <[email protected]> * Apply suggestions from code review Co-authored-by: Guillaume Dalle <[email protected]> * more updates from code-review * format --------- Co-authored-by: akiyuki ishikawa <[email protected]> Co-authored-by: Etienne dg <[email protected]> Co-authored-by: Guillaume Dalle <[email protected]>

gdalle · 2024-04-05T18:53:54Z

Superseded by #353

i-aki-y added 3 commits October 29, 2021 16:13

Add all_simple_paths function

eb24d67

Add a function that finds all simple paths between two nodes in a graph.

Apply review results

a59781e

Fix stack usage

acf0d5a

To improve memory effeciency, make the stack store only parent node and index.

matbesancon reviewed Nov 13, 2021

View reviewed changes

mschauer reviewed Aug 17, 2022

View reviewed changes

gdalle added the enhancement New feature or request label Jun 16, 2023

mschauer reviewed Feb 14, 2024

View reviewed changes

thchr mentioned this pull request Mar 18, 2024

All simple paths (refresh #20) #353

Merged

gdalle closed this Apr 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Port] A proposal of an all_simple_paths function implementation #20

[Port] A proposal of an all_simple_paths function implementation #20

etiennedeg commented Oct 29, 2021

codecov bot commented Nov 7, 2021

jpfairbanks commented Nov 8, 2021

jpfairbanks commented Nov 13, 2021

matbesancon Nov 13, 2021

matbesancon commented Nov 13, 2021

etiennedeg commented Nov 15, 2021

gdalle commented Mar 10, 2022

simonschoelly commented Jul 24, 2022

gdalle commented Jul 24, 2022

simonschoelly commented Jul 24, 2022

gdalle commented Jul 24, 2022

mschauer commented Aug 17, 2022

mschauer Aug 17, 2022

i-aki-y Apr 4, 2023

mschauer commented Aug 17, 2022

i-aki-y commented Apr 4, 2023 •

edited

Loading

mschauer Feb 14, 2024

mschauer commented Feb 14, 2024

mschauer commented Feb 16, 2024

gdalle commented Feb 21, 2024

gdalle commented Apr 5, 2024

		A helper function that updates iterator state.
		For internal use only.

	A helper function that updates iterator state.
	For internal use only.
	A helper function that updates the simple path iterator state.

	all_simple_paths(g, source, targets, cutoff)
	all_simple_paths(g, source, targets; cutoff)

[Port] A proposal of an all_simple_paths function implementation #20

[Port] A proposal of an all_simple_paths function implementation #20

Conversation

etiennedeg commented Oct 29, 2021

codecov bot commented Nov 7, 2021

Codecov Report

jpfairbanks commented Nov 8, 2021

jpfairbanks commented Nov 13, 2021

matbesancon Nov 13, 2021

Choose a reason for hiding this comment

matbesancon commented Nov 13, 2021

etiennedeg commented Nov 15, 2021

gdalle commented Mar 10, 2022

simonschoelly commented Jul 24, 2022

gdalle commented Jul 24, 2022

simonschoelly commented Jul 24, 2022

gdalle commented Jul 24, 2022

mschauer commented Aug 17, 2022

mschauer Aug 17, 2022

Choose a reason for hiding this comment

i-aki-y Apr 4, 2023

Choose a reason for hiding this comment

mschauer commented Aug 17, 2022

i-aki-y commented Apr 4, 2023 • edited Loading

mschauer Feb 14, 2024

Choose a reason for hiding this comment

mschauer commented Feb 14, 2024

mschauer commented Feb 16, 2024

gdalle commented Feb 21, 2024

gdalle commented Apr 5, 2024

i-aki-y commented Apr 4, 2023 •

edited

Loading