Skip to content
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

Implementation of Floyd-Warshall for all pair shortest distance #27518

Closed
Hrishabh-yadav mannequin opened this issue Mar 20, 2019 · 92 comments
Closed

Implementation of Floyd-Warshall for all pair shortest distance #27518

Hrishabh-yadav mannequin opened this issue Mar 20, 2019 · 92 comments

Comments

@Hrishabh-yadav
Copy link
Mannequin

Hrishabh-yadav mannequin commented Mar 20, 2019

Currently sage has implemented Floyd-Warshall algorithm only for graphs with constant edge weight of 1 in cython. Implementation of Floyd warshall algorithm in cython for finding all pair shortest distances(ASAD) can be done. This algorithm can be further improved to find all pair shortest paths(ASAP)(Later).

We can also improve all pair shortest distance algorithm using the technique described in the following paper:-
https://pdfs.semanticscholar.org/568f/58737e752bf831a3e48036cf5205facce769.pdf
Usually complexity for finding ASAD with matrix multiplication comes with large constants that results in slow running time, But the implementation that is described in the paper can faster the running time for algorithm.

CC: @dcoudert

Component: graph theory

Author: Georgios Giapitzakis Tzintanos

Branch/Commit: dc082bb

Reviewer: David Coudert

Issue created by migration from https://trac.sagemath.org/ticket/27518

@Hrishabh-yadav Hrishabh-yadav mannequin added this to the sage-8.8 milestone Mar 20, 2019
@giorgosgiapis
Copy link
Contributor

comment:1

I am working on this. I think we should take care of the case when edge_label is not a number (Probably set weight to 1).

@giorgosgiapis
Copy link
Contributor

@giorgosgiapis
Copy link
Contributor

Commit: ffcea9e

@giorgosgiapis
Copy link
Contributor

New commits:

ffcea9eModified Floyd-Warshall to work with weighted graphs with non-negative integer edge weights.

@dcoudert
Copy link
Contributor

comment:5
  • The usage of edge weights is not fully unified in the graph module. However, many methods have as input parameter wfunction, a function that inputs an edge and outputs a number. You should consider adding this parameter as input of the method.

  • There is an implementation of Floyd-Warshall algorithm in boost https://www.boost.org/doc/libs/1_69_0/libs/graph/doc/floyd_warshall_shortest.html

    and we have an interface with boost src/sage/graphs/base/boost_*.

    So you should first expose the boost implementation in Sagemath.

  • Moreover, Johnson algorithm is considered faster for sparse graphs (and most of the graphs are sparse).

    • we can use the implementation of boost: 'Johnson_Boost'

    • we can also use the implementation of igraph (an optional package), although not directly exposed. In fact method shortest_paths of igraph call Johnson algorithm and not Floyd-Warshall for the reason explained here: https://lists.nongnu.org/archive/html/igraph-help/2009-06/msg00029.html

      when igraph is installed, we can do H = G.igraph_graph() and then H.shortest_paths() with appropriate parameters.

@Hrishabh-yadav
Copy link
Mannequin Author

Hrishabh-yadav mannequin commented Mar 21, 2019

comment:6

Does the algorithm only work if given graph has all possible edges defined, Like if I input a Graph with 100 edges and 11 vertices :

 sage: 
....: from sage.graphs.distances_all_pairs import floyd_warshall
....: G= Graph()
....: G.allow_multiple_edges(True)
....: G.allow_loops(True)
....: G.weighted(True)
....: for i in range(100):
....:     u=randint(0,10)
....:     v=randint(0,10)
....:     l=randint(0,1000)
....:     G.add_edge(u,v,l)
....:     G.add_edge(v,u,l)
....:     
....: print(floyd_warshall(G,paths=False,distances=True))
....: 
....:     
---------------------------------------------------------------------------
LookupError                               Traceback (most recent call last)
<ipython-input-13-c620edb96b9c> in <module>()
     12     G.add_edge(v,u,l)
     13 
---> 14 print(floyd_warshall(G,paths=False,distances=True))
     15 
     16 

/home/hrishabh/sage/local/lib/python2.7/site-packages/sage/graphs/distances_all_pairs.pyx in sage.graphs.distances_all_pairs.floyd_warshall (build/cythonized/sage/graphs/distances_all_pairs.c:15993)()
   1623         for u_int in g.out_neighbors(v_int):
   1624             if gg.weighted() == True:
-> 1625                 label = str(gg.edge_label(v_int, u_int))
   1626                 try:
   1627                     weight = int(label)

/home/hrishabh/sage/local/lib/python2.7/site-packages/sage/graphs/generic_graph.pyc in edge_label(self, u, v)
  11310             True
  11311         """
> 11312         return self._backend.get_edge_label(u,v)
  11313 
  11314     def edge_labels(self):

/home/hrishabh/sage/local/lib/python2.7/site-packages/sage/graphs/base/sparse_graph.pyx in sage.graphs.base.sparse_graph.SparseGraphBackend.get_edge_label (build/cythonized/sage/graphs/base/sparse_graph.c:19894)()
   1643         cdef int v_int = self.get_vertex(v)
   1644         if not (<SparseGraph>self._cg).has_arc_unsafe(u_int, v_int):
-> 1645             raise LookupError("({0}, {1}) is not an edge of the graph.".format(repr(u),repr(v)))
   1646         if self.multiple_edges(None):
   1647             return [self.edge_labels[l_int] if l_int != 0 else None

LookupError: (1, 2) is not an edge of the graph.

But the same algorithm works for 500 edges and 11 vertices:-

....: from sage.graphs.distances_all_pairs import floyd_warshall
....: G= Graph()
....: G.allow_multiple_edges(True)
....: G.allow_loops(True)
....: G.weighted(True)
....: for i in range(500):
....:     u=randint(0,10)
....:     v=randint(0,10)
....:     l=randint(0,1000)
....:     G.add_edge(u,v,l)
....:     G.add_edge(v,u,l)
....:     
....: print(floyd_warshall(G,paths=False,distances=True))
....: 
....:     
{0: {0: 0, 1: 1, 2: 1, 3: 1, 4: 1, 5: 1, 6: 1, 7: 1, 8: 1, 9: 1, 10: 1}, 1: {0: 1, 1: 0, 2: 1, 3: 1, 4: 1, 5: 1, 6: 1, 7: 1, 8: 1, 9: 1, 10: 1}, 2: {0: 1, 1: 1, 2: 0, 3: 1, 4: 1, 5: 1, 6: 1, 7: 1, 8: 1, 9: 1, 10: 1}, 3: {0: 1, 1: 1, 2: 1, 3: 0, 4: 1, 5: 1, 6: 1, 7: 1, 8: 1, 9: 1, 10: 1}, 4: {0: 1, 1: 1, 2: 1, 3: 1, 4: 0, 5: 1, 6: 1, 7: 1, 8: 1, 9: 1, 10: 1}, 5: {0: 1, 1: 1, 2: 1, 3: 1, 4: 1, 5: 0, 6: 1, 7: 1, 8: 1, 9: 1, 10: 1}, 6: {0: 1, 1: 1, 2: 1, 3: 1, 4: 1, 5: 1, 6: 0, 7: 1, 8: 1, 9: 1, 10: 1}, 7: {0: 1, 1: 1, 2: 1, 3: 1, 4: 1, 5: 1, 6: 1, 7: 0, 8: 1, 9: 1, 10: 1}, 8: {0: 1, 1: 1, 2: 1, 3: 1, 4: 1, 5: 1, 6: 1, 7: 1, 8: 0, 9: 1, 10: 1}, 9: {0: 1, 1: 1, 2: 1, 3: 1, 4: 1, 5: 1, 6: 1, 7: 1, 8: 1, 9: 0, 10: 1}, 10: {0: 1, 1: 1, 2: 1, 3: 1, 4: 1, 5: 1, 6: 1, 7: 1, 8: 1, 9: 1, 10: 0}}

Any particular reason?

@giorgosgiapis
Copy link
Contributor

comment:7

Well, I think I figured it out. If we work on the original graph (gg) and not in the graph created in line

cdef CGraph g = <CGraph> gg._backend.c_graph()[0]

the problem seems to get fixed. I don't really know what causes the issue in the first place but in general, the above line seems to add to the graph g some edges which are not present in gg.

@dcoudert
Copy link
Contributor

comment:8
cdef CGraph g = <CGraph> gg._backend.c_graph()[0]

the problem seems to get fixed. I don't really know what causes the issue in the first place but in general, the above line seems to add to the graph g some edges which are not present in gg.

g is in underlying backend graph format. This is a different format than Graph in which nodes and edges have different labels than in the gg. Although you can make much more efficient code using the backend directly, don't start playing with this if you are not an advanced user !

@giorgosgiapis
Copy link
Contributor

comment:9

The boost implementation of Floyd-Warshall and Johnson algorithm returns only the distances and not the paths. Would it be a good idea to use it and retrieve the paths from the distance values? Personally, I can't think of a way to do this faster than O(V3).

@dcoudert
Copy link
Contributor

comment:10

What is your objective: get distances or get paths ? For distances, you can use boost. For paths, this is another story, and you will have to code a specific algorithm. However, I'm not convinced that using Floyd-Warshall is a good idea if you want paths.

@giorgosgiapis
Copy link
Contributor

comment:11

The current implementation has the option to also return the shortest paths between every two vertices. My objective is to preserve this feature. Would it be a good idea to reimplement Johnson's algorithm in Cython to be able to do this efficiently?

@dcoudert
Copy link
Contributor

comment:12

Unless I'm mistaken, the methods don't return paths but the matrix of predecessors.
Given a matrix (or dict of dict) of distances, you can find the predecessors. For a source vertex s, for a vertex v, you find the neighbor u of v that is on a shortest s-v path by checking which vertex u is such that d(s, u) + w(u, v) = d(s, v). So it should be easy to add the functionality to the method implemented with boost.

@giorgosgiapis
Copy link
Contributor

comment:13

Replying to @dcoudert:

Unless I'm mistaken, the methods don't return paths but the matrix of predecessors.
Given a matrix (or dict of dict) of distances, you can find the predecessors. For a source vertex s, for a vertex v, you find the neighbor u of v that is on a shortest s-v path by checking which vertex u is such that d(s, u) + w(u, v) = d(s, v). So it should be easy to add the functionality to the method implemented with boost.

But isn't this going to be O(V3)?

@dcoudert
Copy link
Contributor

comment:14

It will be O(N.M) with a small constants...

@giorgosgiapis
Copy link
Contributor

comment:15

Ok. Here is what I am planning to do in the next couple of days:

  1. Expose the boost implementation of Floyd-Warshall in Sagemath

  2. Modify johnson_shortest_paths() to return the predecessors matrix

  3. Create a new method for all pairs shortest paths where the user will be able to select the algorithm used (like in shortest_paths method in src/sage/graphs/base/boost_graph.pyx).

Please let me know if there are any problems or pitfalls with what I proposed above.

@dcoudert
Copy link
Contributor

comment:16
  1. Expose the boost implementation of Floyd-Warshall in Sagemath

  2. Modify johnson_shortest_paths() to return the predecessors matrix

OK

  1. Create a new method for all pairs shortest paths where the user will be able to select the algorithm used (like in shortest_paths method in src/sage/graphs/base/boost_graph.pyx).

Have you checked the documentation of method shortest_path_all_pairs ? it already returns distances and predecessors matrix for different algorithms.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 30, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

5aceae0Added predecessors support in boost johnson_shortest_paths method
e87b06bMerge branch 'u/gh-giorgosgiapis/weighted_floyd_warshall' of git://trac.sagemath.org/sage into johnson_pred
ef91486Added predecessors support in boost johnson_shortest_paths method

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 30, 2019

Changed commit from ffcea9e to ef91486

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 30, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

41358f9Fixed minor issues

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 30, 2019

Changed commit from ef91486 to 41358f9

@dcoudert
Copy link
Contributor

comment:19
  • please ensure that comments and documentation are in 80 column modes. See the developer manual

  • for building the distance dict, why not using the previous code ?

     dist = {int_to_v[v]: {int_to_v[w]: correct_type(result[v][w])
                     for w in range(N) if result[v][w] != sys.float_info.max}
             for v in range(N)}
  • for building the predecessor dict, it's a pitty not using the boost graph that has already been build and that contains the correct edge weights. I agree it's much more complicated and requires to extend boost_interface.cpp to expose other methods in order to iterate over weighted edges.

    Also, the pseudo code I have in mind is more like

for i, j, w in g.edges():
    for k in range(N):
        if result[k][j] == result[k][i] + w:
            pred[k][j] = i
        if not directed and result[k][i] == result[k][j] + w:
            pred[k][i] = j

It's only the pseudo-code and it must be improved, but it should give a simpler code.

So please check what can be done with boost graphs. It might not be so difficult to use, and should lead to much faster code.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 31, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

8e8799dFixed comments, documentation and code for predecessors

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 31, 2019

Changed commit from 41358f9 to 8e8799d

@giorgosgiapis
Copy link
Contributor

comment:21

Replying to @dcoudert:

  • please ensure that comments and documentation are in 80 column modes. See the developer manual

Fixed all errors

  • for building the distance dict, why not using the previous code ?
     dist = {int_to_v[v]: {int_to_v[w]: correct_type(result[v][w])
                     for w in range(N) if result[v][w] != sys.float_info.max}
             for v in range(N)}

Changed back to the previous code

Also, the pseudo code I have in mind is more like

for i, j, w in g.edges():
    for k in range(N):
        if result[k][j] == result[k][i] + w:
            pred[k][j] = i
        if not directed and result[k][i] == result[k][j] + w:
            pred[k][i] = j

It's only the pseudo-code and it must be improved, but it should give a simpler code.

I adapted the pseudocode to my implementation. I will do some research on boost graphs and try to come up with a faster implementation using boost interface.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 4, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

ef38d91Fixed typo

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 4, 2019

Changed commit from ee720d4 to ef38d91

@dcoudert
Copy link
Contributor

dcoudert commented May 6, 2019

Changed commit from ef38d91 to 803af4e

@dcoudert
Copy link
Contributor

dcoudert commented May 6, 2019

@dcoudert
Copy link
Contributor

dcoudert commented May 6, 2019

comment:57

I have rebased the branch on the last beta version and fixed the merge conflicts.
I pushed a new branch. It is in public/, so you can access and modify it.

Please check that it's working well.


New commits:

803af4etrac #27518: fix merge conflict with 8.8.beta4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

2a26a65trac #27518: alignement

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2019

Changed commit from 803af4e to 2a26a65

@dcoudert
Copy link
Contributor

dcoudert commented May 8, 2019

comment:59

For me this patch is good to go. I'm waiting confirmation from your side.

@giorgosgiapis
Copy link
Contributor

comment:60

Replying to @dcoudert:

For me this patch is good to go. I'm waiting confirmation from your side.

After some testing everything seems to work as expected so it is indeed good to go.

@dcoudert
Copy link
Contributor

dcoudert commented May 9, 2019

Reviewer: David Coudert

@dcoudert
Copy link
Contributor

dcoudert commented May 9, 2019

comment:61

Please add your real name in Authors field.

LGTM.

@giorgosgiapis
Copy link
Contributor

Author: Georgios Giapitzakis Tzintanos

@vbraun
Copy link
Member

vbraun commented May 12, 2019

comment:63

Docbuild fails, see patchbot

@kiwifb
Copy link
Member

kiwifb commented May 13, 2019

comment:64

Just to make things clear, that is the failure I see

Error building the documentation.
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 79, in build_ref_doc
    getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 761, in _wrapper
    getattr(DocBuilder, build_type)(self, *args, **kwds)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 133, in f
    runsphinx()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/sphinxbuild.py", line 317, in runsphinx
    sys.stderr.raise_errors()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/sphinxbuild.py", line 252, in raise_errors
    raise OSError(self._error)
OSError: docstring of sage.graphs.base.boost_graph.johnson_shortest_paths:18: WARNING: Definition list ends without a blank line; unexpected unindent.
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "sage_setup/docbuild/__main__.py", line 2, in <module>
    main()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 1730, in main
    builder()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 351, in _wrapper
    getattr(get_builder(document), 'inventory')(*args, **kwds)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 547, in _wrapper
    build_many(build_ref_doc, L)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 288, in build_many
    ret = x.get(99999)
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 657, in get
    raise self._value
OSError: docstring of sage.graphs.base.boost_graph.johnson_shortest_paths:18: WARNING: Definition list ends without a blank line; unexpected unindent.

it looks like a stray space at the start of line 1085 of sage/graphs/base/boost_graph.pyx https://github.com/sagemath/sagetrac-mirror/blob/develop/src/sage/graphs/base/boost_graph.pyx?id=136cae2e3fb5f6dec8173f427da3321d49b0a835#n1085 is the cause of the problem. Removing it makes the documentation build here.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 13, 2019

Changed commit from 2a26a65 to dc082bb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 13, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

dc082bbFixed typo

@giorgosgiapis
Copy link
Contributor

comment:66

Replying to @kiwifb:

Just to make things clear, that is the failure I see

Error building the documentation.
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 79, in build_ref_doc
    getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 761, in _wrapper
    getattr(DocBuilder, build_type)(self, *args, **kwds)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 133, in f
    runsphinx()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/sphinxbuild.py", line 317, in runsphinx
    sys.stderr.raise_errors()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/sphinxbuild.py", line 252, in raise_errors
    raise OSError(self._error)
OSError: docstring of sage.graphs.base.boost_graph.johnson_shortest_paths:18: WARNING: Definition list ends without a blank line; unexpected unindent.
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "sage_setup/docbuild/__main__.py", line 2, in <module>
    main()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 1730, in main
    builder()
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 351, in _wrapper
    getattr(get_builder(document), 'inventory')(*args, **kwds)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 547, in _wrapper
    build_many(build_ref_doc, L)
  File "/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_7/sage_setup/docbuild/__init__.py", line 288, in build_many
    ret = x.get(99999)
  File "/usr/lib/python3.7/multiprocessing/pool.py", line 657, in get
    raise self._value
OSError: docstring of sage.graphs.base.boost_graph.johnson_shortest_paths:18: WARNING: Definition list ends without a blank line; unexpected unindent.

it looks like a stray space at the start of line 1085 of sage/graphs/base/boost_graph.pyx https://github.com/sagemath/sagetrac-mirror/blob/develop/src/sage/graphs/base/boost_graph.pyx?id=136cae2e3fb5f6dec8173f427da3321d49b0a835#n1085 is the cause of the problem. Removing it makes the documentation build here.

Thank you for the info. Fixed it!

@dcoudert
Copy link
Contributor

comment:67

@kiwifb: can you check if it's ok ? I have an issue building the documentation that is not related to this ticket. Thank you.

@giorgosgiapis
Copy link
Contributor

comment:68

All tests seem to pass now!

@dcoudert
Copy link
Contributor

comment:69

LGTM.

@vbraun
Copy link
Member

vbraun commented May 21, 2019

Changed branch from public/graphs/27518_floyd_warshall_boost to dc082bb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants