Add shortest path algorithm #368
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #368 +/- ##
==========================================
+ Coverage 90.58% 90.74% +0.16%
==========================================
Files 41 42 +1
Lines 3060 3113 +53
==========================================
+ Hits 2772 2825 +53
Misses 288 288
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Overall, looks great @acombretrenouard! Clean code, nice comments, and good testing! My biggest question is on the data structure of the path lengths. Thanks for the contribution!
README.md
Outdated
| @@ -104,3 +104,4 @@ This library may not meet your needs and if this is this case, consider checking | |||
| * [HyperGraphs.jl](https://github.com/lpmdiaz/HyperGraphs.jl): A package in Julia for representing, analyzing, and generating hypergraphs which may be oriented and weighted. | |||
| * [hyperG](https://cran.r-project.org/package=HyperG): A package in R for storing and analyzing hypergraphs | |||
| * [NetworkX](https://networkx.org/): A package in Python for representing, analyzing, and visualizing networks. | |||
| )) | |||
There was a problem hiding this comment.
What's the reason for this line?
There was a problem hiding this comment.
Actually I don't know. I saw it appearing after the pull request.
Just delete it !
xgi/algorithms/shortest_path.py
Outdated
There was a problem hiding this comment.
Looks great. My biggest question would be whether it makes more sense to store the path lengths as a numpy array with a dict mapping node IDs to matrix row/columns.
There was a problem hiding this comment.
I tried to reproduce the signature of shortest_path_length in networkx.
In particular the returns are structured as explained here :
If the source and target are both specified, return the length of the shortest path from the source to the target.
If only the source is specified, return a dict keyed by target to the shortest path length from the source to that target.
If only the target is specified, return a dict keyed by source to the shortest path length from that source to the target.
If neither the source nor target are specified, return an iterator over (source, dictionary) where dictionary is keyed by target to shortest path length from source to that target.
I think returning a generator instead of an array is great for computing shortest paths on-the-fly and saving memory. I guess with time people figured out this was the best way (?).
Thanks for the feedback :)
There was a problem hiding this comment.
Gotcha. Thanks for the explanation! I think I'm good with merging this. And I guess that it only makes sense to take advantage of the
|
I restored |
|
Congrats on your first XGI contribution, @acombretrenouard ! Excited to start computing some shortest paths! 😁 |
|
Thanks @acombretrenouard, and congrats !! What are the details of this generalization to hypergraphs? Are you looking at paths between nodes through hyperedges of any size? When you compute the shortest path, a pairwise edge and a triangle would both count as "length 1" in the path? |
Related to issue #365.
I added a new file ./xgi/algorithms/shortest_path.py containing two functions (
single_source_shortest_pathandshortest_path_length).There is also a new function in ./xgi/utils/utilities.py.
The functions are structured as in
networkxand implement the Disjkstra algorithm.Naming could be improved.
Corresponding test (for algorithms and utilities) and documentation are written. The changelog is up to date as well.
I successfully run
pytest,isort .,pylint xgi/ --disable all --enable W0611andblack ..