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

improvement of the feedback_edge_set method #23989

Closed
dcoudert opened this issue Oct 8, 2017 · 14 comments
Closed

improvement of the feedback_edge_set method #23989

dcoudert opened this issue Oct 8, 2017 · 14 comments

Comments

@dcoudert
Copy link
Contributor

dcoudert commented Oct 8, 2017

As for #23984, we speed up the resolution of the constraint generation formulation adding more constraints per round, i.e., as many constraints as edge disjoint circuits.

Before

sage: D = DiGraph('q??D@O_A_AO?O???a_?a_o??G???CO?_?N_???C?o?aGCc_??O???@?G@??@??_@???@?I?C??G?G@@HC?_?GCAG_??_???????GPo_??????C?Go@??@O?@???GB?@_QA_OAC??W?OOG__OC?C????????cA??G?WGA??_???P???C@A?_??@_IBSD??A?_?Q??o@O????O?C?@_A?O@?G???OS@??G?GAOO??????T??A@A????A???@AO?O??IQ?@L?_?_?C?CG?A??A?DC??AO@@????WOG????`??A???_E_G????S?K?????@?AO?????G?C?A_?G??g?@???O???G?G??A???G_@???D??OA?_OAG??MA?_?A??@?A??C?????OO??????C????O@CG???A??_O')
sage: %timeit D.feedback_edge_set()
1 loop, best of 3: 3.85 s per loop

After

sage: D = DiGraph('q??D@O_A_AO?O???a_?a_o??G???CO?_?N_???C?o?aGCc_??O???@?G@??@??_@???@?I?C??G?G@@HC?_?GCAG_??_???????GPo_??????C?Go@??@O?@???GB?@_QA_OAC??W?OOG__OC?C????????cA??G?WGA??_???P???C@A?_??@_IBSD??A?_?Q??o@O????O?C?@_A?O@?G???OS@??G?GAOO??????T??A@A????A???@AO?O??IQ?@L?_?_?C?CG?A??A?DC??AO@@????WOG????`??A???_E_G????S?K?????@?AO?????G?C?A_?G??g?@???O???G?G??A???G_@???D??OA?_OAG??MA?_?A??@?A??C?????OO??????C????O@CG???A??_O')
sage: %timeit D.feedback_edge_set()
1 loop, best of 3: 1.62 s per loop

This patch also fix an issue for digraphs with loops (see [#23989 comment:5])

Component: graph theory

Author: David Coudert

Branch/Commit: 972edd0

Reviewer: Travis Scrimshaw

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

@dcoudert dcoudert added this to the sage-8.1 milestone Oct 8, 2017
@dcoudert
Copy link
Contributor Author

dcoudert commented Oct 8, 2017

Branch: u/dcoudert/23989

@dcoudert
Copy link
Contributor Author

dcoudert commented Oct 8, 2017

comment:1

I have also added a pre-processing to solve the problem on each strongly connected component.
In addition, we ensure that loops and multiple edges are well handled.


New commits:

8a45f87trac #23989: cleaning original code
7037730trac #23989: add more constraints
ea4d4c9trac #23989: deal with loops
53e701dtrac #23989: doctest for checking that multiedges are properly taken into account
ff893cctrac #23989: decomposition into strongly connected components

@dcoudert
Copy link
Contributor Author

dcoudert commented Oct 8, 2017

Commit: ff893cc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2017

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

6a319fctrac #23989: remove labels of loops

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2017

Changed commit from ff893cc to 6a319fc

@dcoudert
Copy link
Contributor Author

dcoudert commented Oct 8, 2017

comment:3

This method has never returned labeled edges, so I have removed labels of loops.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 9, 2017

Changed commit from 6a319fc to 972edd0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 9, 2017

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

972edd0trac #23989: precision for multigraphs

@dcoudert

This comment has been minimized.

@dcoudert
Copy link
Contributor Author

dcoudert commented Oct 9, 2017

comment:5

I added some comments for graphs with multiple edges (mostly to convince myself that it's correct).

This patch also fix the following issue for graphs with loops.

sage: D = DiGraph(loops=True)
sage: D.add_edge(0,0)
sage: D.feedback_edge_set()
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-81-9e5a629c6d7c> in <module>()
----> 1 D.feedback_edge_set()

/Users/dcoudert/sage/local/lib/python2.7/site-packages/sage/graphs/digraph.pyc in feedback_edge_set(self, constraint_generation, value_only, solver, verbose)
   1573                 for u,v in self.edges(labels = False):
   1574                     if p.get_values(b[u,v]) < .5:
-> 1575                         h.add_edge(u,v)
   1576 
   1577                 # Is the digraph acyclic ?

/Users/dcoudert/sage/local/lib/python2.7/site-packages/sage/graphs/generic_graph.pyc in add_edge(self, u, v, label)
  10416                     pass
  10417 
> 10418         self._backend.add_edge(u, v, label, self._directed)
  10419 
  10420     def add_edges(self, edges, loops=True):

/Users/dcoudert/sage/src/sage/graphs/base/sparse_graph.pyx in sage.graphs.base.sparse_graph.SparseGraphBackend.add_edge (build/cythonized/sage/graphs/base/sparse_graph.c:17249)()
   1488 
   1489         if u_int == v_int and not self._loops:
-> 1490             raise ValueError(f"cannot add edge from {u!r} to {v!r} in graph without loops")
   1491 
   1492         if not self.multiple_edges(None):

ValueError: cannot add edge from 0 to 0 in graph without loops

@tscrim
Copy link
Collaborator

tscrim commented Oct 9, 2017

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Oct 9, 2017

comment:6

LGTM.

@dcoudert
Copy link
Contributor Author

dcoudert commented Oct 9, 2017

comment:7

Thank you very much.

@vbraun
Copy link
Member

vbraun commented Oct 20, 2017

Changed branch from u/dcoudert/23989 to 972edd0

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