-
Notifications
You must be signed in to change notification settings - Fork 66
Implementing DigraphEdgeConnectivity #884
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
571435b
d2fe700
c32aca6
f1ac56e
c6dabf9
c498734
0634eed
7719d52
c0e4068
4f36fde
f15c6ea
462d1a0
9238e88
d3b581f
cfb696c
4f0abdc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1954,6 +1954,46 @@ rec( idom := [ 2, fail, 2, 2, 2 ], preorder := [ 2, 1, 3, 4, 5 ] ) | |||||
| </ManSection> | ||||||
| <#/GAPDoc> | ||||||
|
|
||||||
| <#GAPDoc Label="DigraphDominatingSet"> | ||||||
| <ManSection> | ||||||
| <Oper Name="DigraphDominatingSet" Arg="digraph"/> | ||||||
| <Returns>A List</Returns> | ||||||
| <Description> | ||||||
| This creates a dominating set of <A>digraph</A>, which is a subset of the vertices in | ||||||
| <A>digraph</A> such that the neighbourhood of this subset of vertices is equal to all | ||||||
| the vertices in <A>digraph</A>. </P> | ||||||
|
|
||||||
| The domating set returned will be one of potentially multiple possible dominating sets for the digraph. | ||||||
| <Example><![CDATA[ | ||||||
| D := Digraph([[2,3,4], [],[],[]]);; | ||||||
| gap> DigraphDominatingSet(D); | ||||||
| [ 4, 1 ] | ||||||
| gap> DigraphDominatingSet(D); | ||||||
| [ 1 ]]]></Example> | ||||||
| </Description> | ||||||
| </ManSection> | ||||||
| <#/GAPDoc> | ||||||
|
|
||||||
| <#GAPDoc Label="DigraphGetNeighbourhood"> | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think omitting the Also, since this function is intended for directed graphs, you should specify whether it returns the set of out-neightbours or in-neighbours (and probably implement the in-neighbours version as well). Alternatively, if the function is only intended for symmetric graphs, you should check for this explicitly in the function definition and also document it accordingly. |
||||||
| <ManSection> | ||||||
| <Oper Name="DigraphGetNeighbourhood" Arg="digraph, vertices"/> | ||||||
| <Returns>A List</Returns> | ||||||
| <Description> | ||||||
| This returns the set of vertices that are adjacent to a vertex in <A>vertices</A>, | ||||||
| not including any vertices that exist in <A>vertices</A>. | ||||||
| <Example><![CDATA[ | ||||||
| gap> D := Digraph([[2, 3, 4], [1], [], []]);; | ||||||
| gap> DigraphGetNeighbourhood(D, [1]); | ||||||
| [ 2, 3, 4 ] | ||||||
| gap> DigraphGetNeighbourhood(D, [1, 2]); | ||||||
| [ 3, 4 ] | ||||||
| gap> D := Digraph([[2, 3, 4], [], [], []]);; | ||||||
| gap> DigraphGetNeighbourhood(D, [2]); | ||||||
| [ ]]]></Example> | ||||||
| </Description> | ||||||
| </ManSection> | ||||||
| <#/GAPDoc> | ||||||
|
|
||||||
| <#GAPDoc Label="PartialOrderDigraphMeetOfVertices"> | ||||||
| <ManSection> | ||||||
| <Oper Name="PartialOrderDigraphMeetOfVertices" | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -272,6 +272,31 @@ gap> Sum(flow[1]); | |||||
| </ManSection> | ||||||
| <#/GAPDoc> | ||||||
|
|
||||||
| <#GAPDoc Label="DigraphEdgeConnectivity"> | ||||||
| <ManSection> | ||||||
| <Attr Name="DigraphEdgeConnectivity" Arg="digraph"/> | ||||||
| <Returns>An integer</Returns> | ||||||
| <Description> | ||||||
| This returns a record representing the edge connectivity of <A>digraph</A>.<P/> | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This is very sparse on details. I think you should define what the edge-connectivity of a digraph is, it is not at all clear. Most users reading this function will probably be learning about edge connectivity for the first time, you shouldn't assume they are experts in graph theory (and even experts appreciate the reminder/clarification about what exactly we mean by edge connectivity). As a rule of thumb, when writing, imagine you are writing the doc to explain it to yourself in the past, before you ever undertook this project. Also, a record is a specific object is gap e.g. |
||||||
|
|
||||||
| It makes use of <C>DigraphMaximumFlow(<A>digraph</A>)</C>, by constructing an edge-weighted Digraph | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It might also be a good idea to link to the doc of the |
||||||
| with edge weights of 1, then using the Max-flow min-cut theorem to determine the size of the minimum cut. | ||||||
| For a digraph, the minimum cut is the set of edges that need to be removed to ensure the digraph is | ||||||
| no longer Strongly Connected. </P> | ||||||
| <Example><![CDATA[ | ||||||
| gap> D := Digraph([[2, 3, 4], [1, 3, 4], [1, 2], [2, 3]]);; | ||||||
| gap> DigraphEdgeConnectivity(D); | ||||||
| 2 | ||||||
| gap> D := Digraph([[], [1, 2], [2]]);; | ||||||
| gap> DigraphEdgeConnectivity(D); | ||||||
| 0 | ||||||
| gap> D := Digraph([[1, 2, 3, 4, 5], [1, 2, 3, 4, 5], [1, 2, 3, 4, 5], [1, 2, 3, 4, 5], [1, 2, 3, 4, 5]]);; | ||||||
| gap> DigraphEdgeConnectivity(D); | ||||||
| 4]]></Example> | ||||||
| </Description> | ||||||
| </ManSection> | ||||||
| <#/GAPDoc> | ||||||
|
|
||||||
| <#GAPDoc Label="RandomUniqueEdgeWeightedDigraph"> | ||||||
| <ManSection> | ||||||
| <Oper Name="RandomUniqueEdgeWeightedDigraph" Arg="[filt, ]n[, p]"/> | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2522,6 +2522,45 @@ function(D, root) | |||||||||||||||||||||
| return result; | ||||||||||||||||||||||
| end); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # For calculating a dominating set for a digraph | ||||||||||||||||||||||
| # Algorithm 7 in : | ||||||||||||||||||||||
| # https://www.cse.msu.edu/~cse835/Papers/Graph_connectivity_revised.pdf | ||||||||||||||||||||||
| InstallMethod(DigraphDominatingSet, "for a digraph", | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the method work for all digraphs or only symmetric digraphs? If its only intended for symmetric digraphs, then this should be explicitly checked for in the function and mentioned in the documentation. If its meant to work for all digraphs, it matters whether the dominating set is with respect to in-neighbours or out-neighbours, so the function name should be updated (I think the currently implemented function returns the out-dominating set), say Alternatively, the function could also take the symmetric closure of the digraph, this way it would be a dominating set with respect the notion of undirected adjacency (this might be called a weak dominating set for a directed graph? Not sure). |
||||||||||||||||||||||
| [IsDigraph], | ||||||||||||||||||||||
| function(digraph) | ||||||||||||||||||||||
| local D, neighbourhood, VerticesLeft, Vertices; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Vertices := [1 .. DigraphNrVertices(digraph)]; | ||||||||||||||||||||||
| Shuffle(Vertices); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Shuffling not technically necessary - may be better to just do D := [1]? | ||||||||||||||||||||||
| D := [Vertices[1]]; | ||||||||||||||||||||||
| neighbourhood := DigraphGetNeighbourhood(digraph, D); | ||||||||||||||||||||||
| VerticesLeft := Difference(Vertices, Union(neighbourhood, D)); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| while not IsEmpty(VerticesLeft) do; | ||||||||||||||||||||||
| Append(D, [VerticesLeft[1]]); | ||||||||||||||||||||||
| neighbourhood := DigraphGetNeighbourhood(digraph, D); | ||||||||||||||||||||||
| VerticesLeft := Difference(Vertices, Union(neighbourhood, D)); | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This implementation is faithful to whats written in the notes by Abdol–Hossein Esfahanian, but the description there is tuned for mathematical rigour, but not so much for speed. So this method will be quite slow due to the usage of I think a better way to implement this would be to use some boolean lists ( known as Let Loop though the vertices Doing this will do more or less the same thing as the current while loop, but it will be much faster since setting I'm also not sure if the |
||||||||||||||||||||||
| od; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return D; | ||||||||||||||||||||||
| end); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # For getting the neighbourhood for a given List of Vertices in a Digraph | ||||||||||||||||||||||
| InstallMethod(DigraphGetNeighbourhood, "for a digraph and a List of vertices", | ||||||||||||||||||||||
| [IsDigraph, IsList], | ||||||||||||||||||||||
| function(digraph, vertices) | ||||||||||||||||||||||
| local v, neighbourhood; | ||||||||||||||||||||||
| neighbourhood := []; | ||||||||||||||||||||||
| for v in vertices do | ||||||||||||||||||||||
| neighbourhood := Difference(Union(neighbourhood, | ||||||||||||||||||||||
| OutNeighbours(digraph)[v]), vertices); | ||||||||||||||||||||||
| od; | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is correct, but its performance could be improved. In particular, calling Part of the issue is that the for v in vertices do
UniteSet(neighbourhood, OutNeighbours(digraph)[v]);
DifferenceSet(neighbourhood, vertices);
od;would be a first step to improving the function. But we can do better! Removing the vertices
Suggested change
Here the And then you can remove duplicate vertices using the |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return neighbourhood; | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A small mathematical nuance that crossed my mind, I'm not sure how important it is, but what should we do with loops? I.e. if a vertex
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Reinis, just committed some of your changes :) For this, I didn't take into account any loops since there was no way they would affect the edge connectivity, but would it be better to generalise the function so it does include them? Either way, I implemented the Boolean lists you mentioned so I don't think there's any need for DigraphOutNeighbourhood anymore, I could just remove it!
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Rheya, thank you for making these changes! I spoke to James about this briefly, this seems to be a bit of an edge-case with respect to how it should work with loops. Essentially different users might have their own idea of what should happen, and both are equally mathematically valid depending on the application. So, if we settle on handling this edge case one way or another we may cause subtle bugs inother peoples code down the line. It would be best to either explicitly reject such cases (i.e. disallow vertices with loops in the variable A similar issue came up with the I think for the Thanks again for putting in the work to finish up the pull request! There is also no pressure for you to make all these changes this semester, please feel free to focus on any more pressing matters, like exams etc., if you need to! |
||||||||||||||||||||||
| end); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Computes the fundamental cycle basis of a symmetric digraph | ||||||||||||||||||||||
| # First, notice that the cycle space is composed of orthogonal subspaces | ||||||||||||||||||||||
| # corresponding to the cycle spaces of the connected components. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,10 @@ DeclareGlobalFunction("DIGRAPHS_Edge_Weighted_Dijkstra"); | |
| DeclareOperation("DigraphMaximumFlow", | ||
| [IsDigraph and HasEdgeWeights, IsPosInt, IsPosInt]); | ||
|
|
||
| # Digraph Edge Connectivity | ||
| DeclareOperation("DigraphEdgeConnectivity", [IsDigraph]); | ||
| DeclareOperation("DigraphEdgeConnectivityDS", [IsDigraph]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think its a good idea to have these as two separate functions. In particular, it would be nice to see if one of the two implementations is better than the other and then only include the faster version. Or, if each function is better for a different set of inputs (say one is very good for highly symmetric graphs, but the other works better for random ones), then there is an argument for keeping both, but in this case it would be better for have an optional argument to Did you try to benchmark the two implementations against each other? it would be quite interesting to see if the extra work computing the dominating set is offset by the improvement in performance. |
||
|
|
||
| # 6. Random edge weighted digraphs | ||
| DeclareOperation("RandomUniqueEdgeWeightedDigraph", [IsPosInt]); | ||
| DeclareOperation("RandomUniqueEdgeWeightedDigraph", [IsPosInt, IsFloat]); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -772,6 +772,218 @@ function(D, start, destination) | |||||||||||||||||||||||
| return flows; | ||||||||||||||||||||||||
| end); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ############################################################################# | ||||||||||||||||||||||||
| # Digraph Edge Connectivity | ||||||||||||||||||||||||
| ############################################################################# | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Algorithms constructed off the algorithms detailed in: | ||||||||||||||||||||||||
| # https://www.cse.msu.edu/~cse835/Papers/Graph_connectivity_revised.pdf | ||||||||||||||||||||||||
| # Each Algorithm uses a different method to decrease the time complexity, | ||||||||||||||||||||||||
| # of calculating Edge Connectivity, though all make use of DigraphMaximumFlow() | ||||||||||||||||||||||||
| # due to the Max-Flow, Min-Cut Theorem | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Algorithm 1: Calculating the Maximum Flow of every possible source and sink | ||||||||||||||||||||||||
| # Algorithm 2: Calculating the Maximum Flow to all sinks of an arbitrary source | ||||||||||||||||||||||||
| # Algorithm 3: Finding Maximum Flow within the non-leaves of a Spanning Tree | ||||||||||||||||||||||||
| # Algorithm 4: Constructing a spanning tree with a high number of leaves | ||||||||||||||||||||||||
| # Algorithm 5: Using the spanning tree^ to find Maximum Flow within non-leaves | ||||||||||||||||||||||||
| # Algorithm 6: Finding Maximum Flow within a dominating set of the digraph | ||||||||||||||||||||||||
| # Algorithm 7: Constructing a dominating set for use in Algorithm 6 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Algorithms 4-7 are used below: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # DigraphEdgeConnectivity calculated using Spanning Trees (Algorithm 4 & 5) | ||||||||||||||||||||||||
| InstallMethod(DigraphEdgeConnectivity, "for a digraph", | ||||||||||||||||||||||||
| [IsDigraph], | ||||||||||||||||||||||||
| function(digraph) | ||||||||||||||||||||||||
| local w, weights, EdgeD, VerticesED, min, u, v, | ||||||||||||||||||||||||
| sum, a, b, st, added, NeighboursV, Edges, notadded, | ||||||||||||||||||||||||
| max, NextVertex, notAddedNeighbours, non_leaf; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if DigraphNrVertices(digraph) = 1 then | ||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if DigraphNrStronglyConnectedComponents(digraph) > 1 then | ||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Its better to join the two checks into one.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I was under the impression that the algorithms worked for non symmetric digraphs, which it seems to for the small digraphs I tested on (though admittedly all the larger digraphs I tested on were symmetric anyway since I got them from House of Graphs). I can look at Arc Connectivity when I get some time! Maybe I could swap them out if that's a more useful property to have for digraphs?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Rheya, nevermind, I misread the notes, my previous comment was completely wrong, namely it says:
I previously thought that Chapter 2 was exclusively relating to undirected graphs, but after double checking it looks like I had the wrong idea about it. Ill edit my comments to remove these suggestions. My only hesitation is that Algorithms 4.-7. seem to be written exclusively for undirected graphs, and I'm not sure if the proofs of correctness of these in the literature apply for both directed and undirected graphs. Looking at the following publication by Esfahanian and Hakirn:
they separate the computation of edge-connectivity for graphs and digraphs into two sections and seem to say about the spanning tree-based approach in particular that
And then they describe a different method for digraphs. So it might be that the spanning tree algorithm does not work as written for digraphs. It might be simplest to restrict the method to symmetric graphs for now. If it turns out that it also words for non-symmetric ones, we can always expand it in the future. I don't think you need to implement arc connectivity, it seems to be quite different from the method for edge-connectivity. |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| weights := List([1 .. DigraphNrVertices(digraph)], | ||||||||||||||||||||||||
| x -> List([1 .. Length(OutNeighbours(digraph)[x])], | ||||||||||||||||||||||||
| y -> 1)); | ||||||||||||||||||||||||
| EdgeD := EdgeWeightedDigraph(digraph, weights); | ||||||||||||||||||||||||
| VerticesED := [1 .. DigraphNrVertices(EdgeD)]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| min := -1; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Algorithm 4: Constructing a Spanning Tree with a large numbers of leaves | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| st := EmptyDigraph(DigraphNrVertices(EdgeD)); | ||||||||||||||||||||||||
| v := 1; | ||||||||||||||||||||||||
| added := [v]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| while DigraphNrEdges(st) < DigraphNrVertices(EdgeD) - 1 do | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Add all edges incident from v | ||||||||||||||||||||||||
| NeighboursV := OutNeighbors(EdgeD)[v]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Edges := List([1 .. Length(NeighboursV)], | ||||||||||||||||||||||||
| x -> [v, OutNeighbours(EdgeD)[v][x]]); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Edges := Difference(Edges, [[v, v]]); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| st := DigraphAddEdges(st, Edges); | ||||||||||||||||||||||||
| Append(added, Difference(OutNeighbours(EdgeD)[v], added)); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Select the neighbour to v with the highest number of not-added neighbours: | ||||||||||||||||||||||||
| notadded := Difference(VerticesED, added); | ||||||||||||||||||||||||
| max := 0; | ||||||||||||||||||||||||
| NextVertex := v; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Preventing infinite iteration if the current vertex has no neighbours | ||||||||||||||||||||||||
| if (Length(NeighboursV) = 0) then | ||||||||||||||||||||||||
| # Pick from a vertex that hasn't been added yet | ||||||||||||||||||||||||
| NextVertex := notadded[1]; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| for w in Difference(NeighboursV, [v]) do; | ||||||||||||||||||||||||
| notAddedNeighbours := Intersection(notadded, OutNeighbours(EdgeD)[w]); | ||||||||||||||||||||||||
| if (Length(notAddedNeighbours) > max) then | ||||||||||||||||||||||||
| max := Length(notAddedNeighbours); | ||||||||||||||||||||||||
| NextVertex := w; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
| od; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| v := NextVertex; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| od; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Algorithm 5: Iterating through the non-leaves of the | ||||||||||||||||||||||||
| # Spanning Tree created in Algorithm 4 to find the Edge Connectivity | ||||||||||||||||||||||||
| non_leaf := []; | ||||||||||||||||||||||||
| for b in VerticesED do | ||||||||||||||||||||||||
| if not IsEmpty(OutNeighbours(st)[b]) then | ||||||||||||||||||||||||
| Append(non_leaf, [b]); | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
| od; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (Length(non_leaf) > 1) then | ||||||||||||||||||||||||
| u := non_leaf[1]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| for v in [2 .. Length(non_leaf)] do | ||||||||||||||||||||||||
| a := DigraphMaximumFlow(EdgeD, u, non_leaf[v])[u]; | ||||||||||||||||||||||||
| b := DigraphMaximumFlow(EdgeD, non_leaf[v], u)[non_leaf[v]]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| sum := Minimum(Sum(a), Sum(b)); | ||||||||||||||||||||||||
| if (sum < min or min = -1) then | ||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Unlike in C++, the brackets are not required for conditionals in if statements. |
||||||||||||||||||||||||
| min := sum; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (sum = 0) then | ||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
| od; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| min := Minimum(min, Minimum(Minimum(OutDegrees(EdgeD)), | ||||||||||||||||||||||||
| Minimum(InDegrees(EdgeD)))); | ||||||||||||||||||||||||
| else | ||||||||||||||||||||||||
| # In the case of spanning trees with only one non-leaf node, | ||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense here to try the dominating set method and only use the simpler all-pairs method if that one fails too? This would make the logic more complicated so just a suggestion. |
||||||||||||||||||||||||
| # the above algorithm does not work | ||||||||||||||||||||||||
| # Revert to iterating through all vertices of the original digraph | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| u := 1; | ||||||||||||||||||||||||
| for v in [2 .. DigraphNrVertices(EdgeD)] do | ||||||||||||||||||||||||
| a := DigraphMaximumFlow(EdgeD, u, v)[u]; | ||||||||||||||||||||||||
| b := DigraphMaximumFlow(EdgeD, v, u)[v]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| sum := Minimum(Sum(a), Sum(b)); | ||||||||||||||||||||||||
| if (sum < min or min = -1) then | ||||||||||||||||||||||||
| min := sum; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (sum = 0) then | ||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| od; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (min = -1) then | ||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return min; | ||||||||||||||||||||||||
| end); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Digraph EdgeConnectivity calculated with Dominating Sets (Algorithm 6-7) | ||||||||||||||||||||||||
| InstallMethod(DigraphEdgeConnectivityDS, "for a digraph", | ||||||||||||||||||||||||
| [IsDigraph], | ||||||||||||||||||||||||
| function(digraph) | ||||||||||||||||||||||||
| # Form an identical but edge weighted digraph with all edge weights as 1: | ||||||||||||||||||||||||
| local weights, i, u, v, w, neighbourhood, EdgeD, | ||||||||||||||||||||||||
| maxFlow, min, sum, a, b, V, added, st, non_leaf, max, | ||||||||||||||||||||||||
| notAddedNeighbours, notadded, NextVertex, NeighboursV, | ||||||||||||||||||||||||
| neighbour, Edges, D, VerticesLeft, VerticesED; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if DigraphNrVertices(digraph) = 1 then | ||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if DigraphNrStronglyConnectedComponents(digraph) > 1 then | ||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| weights := List([1 .. DigraphNrVertices(digraph)], | ||||||||||||||||||||||||
| x -> List([1 .. Length(OutNeighbours(digraph)[x])], | ||||||||||||||||||||||||
| y -> 1)); | ||||||||||||||||||||||||
| EdgeD := EdgeWeightedDigraph(digraph, weights); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| min := -1; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Algorithm 7: Creating a dominating set of the digraph | ||||||||||||||||||||||||
| D := DigraphDominatingSet(digraph); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Algorithm 6: Using the dominating set created to determine the Maximum Flow | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if Length(D) > 1 then | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| v := D[1]; | ||||||||||||||||||||||||
| for i in [2 .. Length(D)] do | ||||||||||||||||||||||||
| w := D[i]; | ||||||||||||||||||||||||
| a := DigraphMaximumFlow(EdgeD, v, w)[v]; | ||||||||||||||||||||||||
| b := DigraphMaximumFlow(EdgeD, w, v)[w]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| sum := Minimum(Sum(a), Sum(b)); | ||||||||||||||||||||||||
| if (sum < min or min = -1) then | ||||||||||||||||||||||||
| min := sum; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
| od; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| else | ||||||||||||||||||||||||
| # If the dominating set of EdgeD is of Length 1, | ||||||||||||||||||||||||
| # the above algorithm will not work | ||||||||||||||||||||||||
| # Revert to iterating through all vertices of the original digraph | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| u := 1; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| for v in [2 .. DigraphNrVertices(EdgeD)] do | ||||||||||||||||||||||||
| a := DigraphMaximumFlow(EdgeD, u, v)[u]; | ||||||||||||||||||||||||
| b := DigraphMaximumFlow(EdgeD, v, u)[v]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| sum := Minimum(Sum(a), Sum(b)); | ||||||||||||||||||||||||
| if (sum < min or min = -1) then | ||||||||||||||||||||||||
| min := sum; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| od; | ||||||||||||||||||||||||
| fi; | ||||||||||||||||||||||||
|
Comment on lines
+841
to
+858
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part of code is almost the same across the two |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return Minimum(min, | ||||||||||||||||||||||||
| Minimum(Minimum(OutDegrees(EdgeD)), | ||||||||||||||||||||||||
| Minimum(InDegrees(EdgeD)))); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| end); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ############################################################################# | ||||||||||||||||||||||||
| # 6. Random edge weighted digraphs | ||||||||||||||||||||||||
| ############################################################################# | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions for rephrasing the sentence a bit. I think generally the first sentence of a function description should concisely describe what the function does in terms of what does the function expect of its arguments and what does the function return. Saying it "creates" something is not bad, but its maybe slightly unclear whether this is created internally and stored somewhere or returned by the function directly etc. Also the new paragraph attribute should be
<P/>.I think you should also clarify what a neighbourhood of a set of vertices is, this is not immediately clear.