Conversation
| if not IsDigraph(G) then | ||
| Error("Input must be a digraph"); | ||
| elif not IsStronglyConnectedDigraph(G) then |
There was a problem hiding this comment.
| if not IsDigraph(G) then | |
| Error("Input must be a digraph"); | |
| elif not IsStronglyConnectedDigraph(G) then | |
| if not IsStronglyConnectedDigraph(G) then |
The first if clause is redundant.
| if not IsDigraph(G) then | ||
| Error("Input must be a digraph"); | ||
| elif not IsStronglyConnectedDigraph(G) then | ||
| Error("Input digraph is not strongly connected; property undefined"); |
There was a problem hiding this comment.
| Error("Input digraph is not strongly connected; property undefined"); | |
| ErrorNoReturn("the argument <G> (a digraph) must be strongly connected"); |
There was a problem hiding this comment.
And please make a similar change for the error message a few lines above.
| Error("Input digraph is not strongly connected; property undefined"); | ||
| fi; | ||
| ecc := []; | ||
| for u in [1 .. DigraphNrVertices(G)] do |
There was a problem hiding this comment.
| for u in [1 .. DigraphNrVertices(G)] do | |
| for u in DigraphVertices(G) do |
| ecc := []; | ||
| for u in [1 .. DigraphNrVertices(G)] do | ||
| c := 0; | ||
| for v in [1 .. DigraphNrVertices(G)] do |
There was a problem hiding this comment.
| for v in [1 .. DigraphNrVertices(G)] do | |
| for v in DigraphVertices(G) do |
|
|
||
| InstallMethod(DigraphDistanceMetrics, "for a digraph", | ||
| [IsDigraph], | ||
| function(G) |
There was a problem hiding this comment.
I think G should be changed to D everywhere here, for consistency.
| od; | ||
| Add(ecc, c); | ||
| od; | ||
| return rec( |
There was a problem hiding this comment.
I'm not 100% sure but I think the following is not particularly efficient because the computation of Minimum(ecc) and Maximum(ecc) will be repeated for each distinct value of i which is not such a good idea. Better to compute these values prior to this return statement.
|
This PR is also missing any documentation at all, which makes it not mergeable at the moment. |
james-d-mitchell
left a comment
james-d-mitchell
left a comment
There was a problem hiding this comment.
Requires some changes before this can be merged.
| if not IsDigraph(G) then | ||
| Error("Input must be a digraph"); | ||
| elif not IsStronglyConnectedDigraph(G) then | ||
| Error("Input digraph is not strongly connected; property undefined"); |
There was a problem hiding this comment.
And please make a similar change for the error message a few lines above.
3 participants
Following my comparison of features in Sage vs. GAP, I added functions computing the radius, periphery, and centre of a strongly connected digraph (
DigraphRadius,DigraphPeriphery,DigraphCentre).Could be merged with
DiagraphDiameter.