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

Python max flow method #9350

Closed
nathanncohen mannequin opened this issue Jun 27, 2010 · 15 comments
Closed

Python max flow method #9350

nathanncohen mannequin opened this issue Jun 27, 2010 · 15 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jun 27, 2010

Implementation of a max-flow algorithm which does not use Linear Programming. A tad faster than the current LP implementation.

This ticket also updates several other methods which formerly used flows (or could be made to), so that they may use the speedup !

sage: g = graphs.PetersenGraph()
sage: %timeit g.flow(0,1, method = "LP")
125 loops, best of 3: 2.85 ms per loop
sage: %timeit g.flow(0,1)
625 loops, best of 3: 1.19 ms per loop
sage: g = graphs.RandomGNP(200,0.1)
sage: %timeit g.flow(0,1, method = "LP")
5 loops, best of 3: 322 ms per loop
sage: %timeit g.flow(0,1)
5 loops, best of 3: 73.5 ms per loop
sage: %timeit g.edge_cut(0,1, method="LP")
5 loops, best of 3: 377 ms per loop
sage: %timeit g.edge_cut(0,1)
5 loops, best of 3: 72.5 ms per loop
g = graphs.RandomGNP(50,0.2)
sage: %timeit g.gomory_hu_tree()
5 loops, best of 3: 4.22 s per loop
sage: %timeit g.gomory_hu_tree(method="LP")
5 loops, best of 3: 5.38 s per loop

I expected a much better speedup for gomory_hu, by the way.... It's odd, it sounds like the bottleneck is somewhere else... O_o

This patch is dedicated to anybody who ever refused one of my patches for lack of doctests. I wouldn't have noticed half of the bugs in this patch without the help of those doctests in the functions that use flow... I won't ever complain anymore because of that ! :-D

Nathann

Component: graph theory

Author: Nathann Cohen

Reviewer: Dmitrii Pasechnik, David Joyner

Merged: sage-4.6.alpha1

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

@nathanncohen nathanncohen mannequin added this to the sage-4.6 milestone Jun 27, 2010
@nathanncohen

This comment has been minimized.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jul 8, 2010

comment:2

Updated, and now needs review :-)

@nathanncohen

This comment has been minimized.

@nathanncohen nathanncohen mannequin added the s: needs review label Jul 8, 2010
@wdjoyner
Copy link

comment:3

This applies okay to 4.5.2.a1 and seems to pass all tests, except for some unrelated failures. However, there are no examples in edge_cut with vertices=True, eg

sage: g = graphs.PetersenGraph()
sage: g.edge_cut(0, 3, method="FF", vertices=True)
[3, [(0, 1, None), (0, 4, None), (0, 5, None)], [[0], [1, 2, 3, 4, 5, 6, 7, 8, 9]]]

Why is this?

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jul 29, 2010

comment:4

Why is this?

What do you think of line 3652 of the generic_graph.py file ? :-)

sage: value,edges,[set1,set2] = g.edge_cut(0, 14, use_edge_labels=True, vertices=True) 

Nathann

@wdjoyner
Copy link

comment:5

Replying to @nathanncohen:

Why is this?

What do you think of line 3652 of the generic_graph.py file ? :-)

sage: value,edges,[set1,set2] = g.edge_cut(0, 14, use_edge_labels=True, vertices=True) 

Nathann

The value isn't returned, so the potential user cannot see examples of how the output changes for different choices of the input. I'm just wondering if there was a good reason for omitting the edges in the output. An example with value_only=False would be equally nice. Finally, to be extremely picky, the description

``vertices`` -- boolean (default: ``False``). When set to ``True``,
          also returns the two sets of vertices that are disconnected by
          the cut. Implies ``value_only=False``.

should probably read

``vertices`` -- boolean (default: ``False``). When set to ``True``,
          returns a list of edges in the edge cut and the two 
          sets of vertices that are disconnected by the cut. 
          Note: ``vertices=True`` implies ``value_only=False``.

Does this seem reasonable?

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jul 29, 2010

Attachment: trac_9350.patch.gz

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jul 29, 2010

comment:6

It does ! I just updated the patch with you example and the corrected docstring :-)

There was, indeed, a reason for never showing directly the output of all these methods : it formerly used Linear Programming, and the results of LP, eve though correct, can vary depending on the time of the day and the solver used. So showing it is asking for trouble, though one can perfectly check some relations... But this Python implementation being deterministic, it's fine now !

Nathann

@wdjoyner
Copy link

comment:7

Replying to @nathanncohen:

It does ! I just updated the patch with you example and the corrected docstring :-)

There was, indeed, a reason for never showing directly the output of all these
methods : it formerly used Linear Programming, and the results of LP, even
though correct, can vary depending on the time of the day and the solver used.
So showing it is asking for trouble, though one can perfectly check some
relations... But this Python implementation being deterministic, it's fine now !

Nathann

Excellent. Passed tested for me (except for unrelated doctest failures on a mac 10.6.4).

Thanks Nathann!!

@dimpase
Copy link
Member

dimpase commented Sep 4, 2010

comment:8

I also tested this, and it looks OK.
Wdj, have you forgotten to give it positive review?

Dima

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Sep 4, 2010

comment:9

Thanksssss ! :-)

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 15, 2010

Author: Nathann Cohen

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 15, 2010

Reviewer: Dmitrii Pasechnik, David Joyner

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 15, 2010

comment:11

Please remember to fill in the "Author(s)" and "Reviewer(s)" fields.

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 15, 2010

Merged: sage-4.6.alpha1

@qed777 qed777 mannequin removed the s: positive review label Sep 15, 2010
@qed777 qed777 mannequin closed this as completed Sep 15, 2010
@nathanncohen nathanncohen mannequin mentioned this issue Sep 15, 2010
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