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

Ford Fulkerson algorithm does not handle unconnected vertices correctly + unclear error message + lacks tests #24925

Closed
sagetrac-tmonteil mannequin opened this issue Mar 7, 2018 · 10 comments

Comments

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Mar 7, 2018

As reported on this ask question, the _ford_fulkerson method for graphs does not handle unconnected vertices correctly:

sage: G = Graph({0:[],1:[]})
sage: G.flow(0,1, algorithm='FF')
ValueError: vertex '0' is not in the (di)graph

To be compared to:

sage: G.flow(0,1, algorithm='LP')
0.0
sage: G.flow(0,1, algorithm='igraph') # depends on python_igraph
0.0

Moreover, the error message is misleading since the vertex is here:

sage: G.vertices()
[0, 1]

This is because the test is about some residual auxiliary graph, not self.

Also, this method lacks test, there are much less than the various proposed options.

Component: graph theory

Keywords: digraph

Author: David Coudert

Branch/Commit: 0d39e1d

Reviewer: Darij Grinberg

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

@sagetrac-tmonteil sagetrac-tmonteil mannequin added this to the sage-8.2 milestone Mar 7, 2018
@dcoudert
Copy link
Contributor

Author: David Coudert

@dcoudert
Copy link
Contributor

Commit: 61330f2

@dcoudert
Copy link
Contributor

Branch: u/dcoudert/24925_ford_fulkerson

@dcoudert
Copy link
Contributor

comment:1

The isolated vertices where ignored in the construction of the residual graph. Easy to fix.


New commits:

61330f2trac #24925: add vertices to residual graph

@darijgr
Copy link
Contributor

darijgr commented Mar 31, 2018

Changed branch from u/dcoudert/24925_ford_fulkerson to public/ticket/24925

@darijgr
Copy link
Contributor

darijgr commented Mar 31, 2018

Changed keywords from none to digraph

@darijgr
Copy link
Contributor

darijgr commented Mar 31, 2018

Reviewer: Darij Grinberg

@darijgr
Copy link
Contributor

darijgr commented Mar 31, 2018

Changed commit from 61330f2 to 0d39e1d

@darijgr
Copy link
Contributor

darijgr commented Mar 31, 2018

comment:2

LGTM, thanks for noticing the bug!

@vbraun
Copy link
Member

vbraun commented May 12, 2018

Changed branch from public/ticket/24925 to 0d39e1d

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

3 participants