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

shortest_path_all pairs in Cython through Floyd Warshall #7676

Closed
nathanncohen mannequin opened this issue Dec 13, 2009 · 17 comments
Closed

shortest_path_all pairs in Cython through Floyd Warshall #7676

nathanncohen mannequin opened this issue Dec 13, 2009 · 17 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 13, 2009

Everything is explained there :

http://en.wikipedia.org/wiki/Floyd%E2%80%93Warshall_algorithm

Component: graph theory

Author: David Coudert

Branch/Commit: 12d4018

Reviewer: Dima Pasechnik

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

@nathanncohen nathanncohen mannequin added this to the sage-5.11 milestone Dec 13, 2009
@nathanncohen nathanncohen mannequin assigned rlmill Dec 13, 2009
@nathanncohen nathanncohen mannequin added the s: needs work label Jun 6, 2010
@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@embray
Copy link
Contributor

embray commented Jan 7, 2019

comment:6

According to https://ask.sagemath.org/question/44823/sage-floyd-algorithm-in-cython/ SciPy already includes an implementation of this that is quite fast, and should probably be used over any implementation in Sage.

@embray embray removed this from the sage-6.4 milestone Jan 7, 2019
@vipul79321
Copy link
Mannequin

vipul79321 mannequin commented Feb 6, 2020

comment:7

If nobody else is currently working on this can I go ahead and implement shortest_path_all pair using Floyd warshall algorithm ?

@dcoudert
Copy link
Contributor

dcoudert commented Feb 7, 2020

comment:8

The first step is to survey the many algorithms we already have with data structures and specific conditions (e.g., weighted, multi edges, loops, etc.).

@vipul79321
Copy link
Mannequin

vipul79321 mannequin commented Feb 8, 2020

comment:9

I have surveyed all the algorithm and data structure sage has. All pair shortest path works only for an unweighted graph. Although we can use boost interface for this purpose. But I think sage should have its own implementation of Floyd Warshall algorithmhttps://en.wikipedia.org/wiki/Floyd%E2%80%93Warshall_algorithm for a weighted graph with positive or negative edge weights(but with no negative cycles).

@dcoudert
Copy link
Contributor

dcoudert commented Feb 8, 2020

comment:10

I don't think sage should have it's own implementation if the one of Boost is fast enough.
You can give it a try, but I'm not sure you can produce something faster than 'Floyd-Warshall_Boost'.

Note that we already have a (slow) Python implementation 'Floyd-Warshall-Python' for weighted graphs, and a Cython implementation 'Floyd-Warshall-Cython' but for unweighted graphs only.

@vipul79321
Copy link
Mannequin

vipul79321 mannequin commented Feb 8, 2020

comment:11

Thanks for your helpful suggestion. I will look into it.

@dcoudert
Copy link
Contributor

comment:12

I added the Floyd-Warshall implementation of SciPy. It's working well but it's not as fast as expected. In fact, most of the time is wasted in turning the graph to a Numpy array and then turning the results to required dict of dicts.

sage: D = digraphs.RandomDirectedGNP(30, .33)
sage: for u, v in D.edges(labels=False):
....:     D.set_edge_label(u, v, randrange(100))
sage: %time _ = D.shortest_path_all_pairs(by_weight=True, algorithm='Floyd-Warshall_Boost')
CPU times: user 6.44 ms, sys: 517 µs, total: 6.96 ms
Wall time: 6.56 ms
sage: %time _ = D.shortest_path_all_pairs(by_weight=True, algorithm='Floyd-Warshall_SciPy')
CPU times: user 30.2 ms, sys: 1.85 ms, total: 32 ms
Wall time: 30.6 ms
sage: D = digraphs.RandomDirectedGNP(300, .33)
sage: for u, v in D.edges(labels=False):
....:     D.set_edge_label(u, v, randrange(100))
sage: %time _ = D.shortest_path_all_pairs(by_weight=True, algorithm='Floyd-Warshall_Boost')
CPU times: user 2.73 s, sys: 18.6 ms, total: 2.75 s
Wall time: 2.76 s
sage: %time _ = D.shortest_path_all_pairs(by_weight=True, algorithm='Floyd-Warshall_SciPy')
CPU times: user 1.95 s, sys: 61.6 ms, total: 2.01 s
Wall time: 2.13 s

New commits:

12d4018trac #7676: Floyd-Warshall with SciPy

@dcoudert
Copy link
Contributor

Branch: public/graphs/7676_floyd_scipy

@dcoudert
Copy link
Contributor

Author: David Coudert

@dcoudert
Copy link
Contributor

Commit: 12d4018

@dcoudert dcoudert added this to the sage-9.5 milestone Oct 16, 2021
@dimpase
Copy link
Member

dimpase commented Dec 4, 2021

comment:13

So scipy thinks of graphs as matrices, there no special format/backend?

@dcoudert
Copy link
Contributor

dcoudert commented Dec 4, 2021

comment:14

When you look at the code of scipy (for instance, https://github.com/scipy/scipy/blob/master/scipy/sparse/csgraph/_shortest_path.pyx), the input parameter is always

    csgraph : array, matrix, or sparse matrix, 2 dimensions
        The N x N array of distances representing the input graph.

You can also check method validate_graph in https://github.com/scipy/scipy/blob/master/scipy/sparse/csgraph/_validation.py. It may convert from one matrix format to another.

@dcoudert
Copy link
Contributor

dcoudert commented Dec 4, 2021

comment:15

So now the question is whether we finalize this ticket or move it to wontfix. Both options are OK for me.

@dimpase
Copy link
Member

dimpase commented Dec 4, 2021

comment:16

let us merge it

@dimpase
Copy link
Member

dimpase commented Dec 4, 2021

Reviewer: Dima Pasechnik

@dcoudert
Copy link
Contributor

dcoudert commented Dec 4, 2021

comment:18

Thanks.

@vbraun
Copy link
Member

vbraun commented Dec 12, 2021

Changed branch from public/graphs/7676_floyd_scipy to 12d4018

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

5 participants