Resolved comments on original function.

Author: finnbuck

gap/oper.gi

@@ -797,82 +797,54 @@ InstallMethod(AmalgamDigraphs,
 "for a digraph, a digraph, a list, and a list",
 [IsDigraph, IsDigraph, IsList, IsList],
 function(D1, D2, subdigraphVertices1, subdigraphVertices2)
-  local D, map, vertex, vertexList, size, iterator, edgeList, edge;
+  local D, n, map, T, vertex, edge;
 
   if not InducedSubdigraph(DigraphImmutableCopyIfMutable(D1),
-         subdigraphVertices1) = 
+         subdigraphVertices1) =
          InducedSubdigraph(DigraphImmutableCopyIfMutable(D2),
          subdigraphVertices2) then
     ErrorNoReturn(
-      "the subdigraph induced by the 3rd argument (a list) in the 1st ",
-      "argument (a digraph) does not equal the subdigraph induced by the ",
-      "4th argument (a list) in the 2nd argument (a digraph)");
+        "the subdigraph induced by the 3rd argument (a list) in the 1st ",
+        "argument (a digraph) does not equal the subdigraph induced by the ",
+        "4th argument (a list) in the 2nd argument (a digraph)");
   fi;
 
-  # Create a mutable copy so that the function also works on
-  # immutable input digraphs and also does not change mutable
-  # digraphs in place.
-  D := DigraphMutableCopy(D1);
+  # Create a mutable copy so that the function also works if
+  # D1 is immutable. If D1 is a mutable digraph then
+  # D1 will be changed in place.
+  D := DigraphMutableCopyIfImmutable(D1);
+
+  n := DigraphNrVertices(D2)
+         + DigraphNrVertices(D1)
+         - Length(subdigraphVertices1);
 
   # 'map' is a mapping from the vertices of D2 to the vertices of the
   # final output graph. The idea is to map the subdigraph vertices of D2
   # onto the subdigraph vertices of D1 and then map the rest of the vertices
   # of D2 to other (higher) values. The mapping from D1 to the output graph
   # can be understood as the identity mapping.
-  map := [1 .. DigraphNrVertices(D2)];
+
+  map := [1 .. n];
 
   for vertex in [1 .. Length(subdigraphVertices1)] do
     map[subdigraphVertices2[vertex]] := subdigraphVertices1[vertex];
   od;
 
-  # Delete??
-  # vertexList := Difference(DigraphVertices(D2), subdigraphVertices2);
-  # size := DigraphNrVertices(D1);
-  # iterator := 1;
-  # for vertex in vertexList do
-  #   map[vertex] := iterator + size;
-  #   iterator := iterator + 1;
-  # od;
-
-
-  iterator := 1;
-  for edge in [1 .. DigraphNrEdges(D2)] do
-    Add(edgeList, []);
-    for vertex in [1, 2] do
-      if DigraphEdges(D2)[edge][vertex] in subdigraphVertices2 then
-        DigraphEdges(D2)[edge][vertex] := DigraphEdges(D2)[edge][vertex] ^ Transformation(map);
-      else
-        DigraphEdges(D2)[edge][vertex] := # But we still need a mapping for the non-subdigraph vertices
-      fi;
-    od;
-  od;
+  map{Difference(DigraphVertices(D2), subdigraphVertices2)} :=
+        [DigraphNrVertices(D1) + 1 .. n];
 
+  T := Transformation(map);
 
+  DigraphAddVertices(D, DigraphNrVertices(D2) - Length(subdigraphVertices1));
 
-  # The problem with adding edges to the output graph was that the
-  # edges of the subdigraph were added twice, creating multiple
-  # edges between certain pairs of points. A quick and readable fix
-  # would have been to use DigraphRemoveAllMultipleEdges, but I decided
-  # to check each of the edges being added to see if they were already
-  # in the subdigraph. This way the function does not end up adding edges
-  # only to delete them later.
-  
-  # edgeList := ShallowCopy(DigraphEdges(D2));
-  # iterator := 1;
-  # while iterator <= Length(edgeList) do
-  #   if edgeList[iterator][1] in subdigraphVertices2 and
-  #       edgeList[iterator][2] in subdigraphVertices2 then
-  #     Remove(edgeList, iterator);
-  #   else
-  #     edgeList[iterator] := [
-  #       map.(edgeList[iterator][1]), map.(edgeList[iterator][2])];
-  #     iterator := iterator + 1;
-  #   fi;
-  # od;
+  for edge in DigraphEdges(D2) do
+    if not (edge[1] in subdigraphVertices2
+        and edge[2] in subdigraphVertices2) then
+      DigraphAddEdge(D, [edge[1] ^ T, edge[2] ^ T]);
+    fi;
+  od;
 
-  DigraphAddVertices(D, DigraphNrVertices(D2) - Length(subdigraphVertices1));
-  DigraphAddEdges(D, edgeList);
-  return [Immutable(DigraphRemoveAllMultipleEdges(D)), map];
+  return [MakeImmutable(D), T];
 end);
 
 ###############################################################################

tst/standard/oper.tst

@@ -2762,24 +2762,24 @@ gap> D1 := Digraph([[2, 3], [1, 3], [1, 2], [2], [3, 4]]);;
 gap> D2 := Digraph([[2, 6], [1, 3, 5], [4], [3], [4, 6], [1, 5]]);;
 gap> U := AmalgamDigraphs(D1, D2, [2, 3, 4, 5], [4, 3, 5, 2]);
 [ <immutable digraph with 7 vertices, 15 edges>, 
-  rec( 1 := 6, 2 := 5, 3 := 3, 4 := 2, 5 := 4, 6 := 7 ) ]
+  Transformation( [ 6, 5, 3, 2, 4, 7, 7 ] ) ]
 gap> D1 := Digraph([
 >   [2, 3], [1, 3, 4, 6], [1, 2, 5, 7], [2, 6], [3, 7], [2, 4, 7, 8],
 >   [3, 5, 6, 8], [6, 7]]);;
 gap> D2 := Digraph([
 >   [2, 3], [1, 4], [1, 5], [2, 5, 6], [3, 4, 7], [4, 7], [5, 6]]);;
 gap> U := AmalgamDigraphs(D1, D2, [2, 3, 6, 7], [4, 5, 6, 7]);
 [ <immutable digraph with 11 vertices, 32 edges>, 
-  rec( 1 := 9, 2 := 10, 3 := 11, 4 := 2, 5 := 3, 6 := 6, 7 := 7 ) ]
+  Transformation( [ 9, 10, 11, 2, 3, 6, 7, 8, 9, 10, 11 ] ) ]
 gap> AmalgamDigraphs(D1, D2, [3, 6, 2, 7], [4, 5, 7, 6]);
 Error, the subdigraph induced by the 3rd argument (a list) in the 1st argument\
  (a digraph) does not equal the subdigraph induced by the 4th argument (a list\
 ) in the 2nd argument (a digraph)
 gap> D1 := PetersenGraph();;
 gap> U := AmalgamDigraphs(D1, D1, [3, 4, 6, 8, 9], [3, 4, 6, 8, 9]);
 [ <immutable digraph with 15 vertices, 50 edges>, 
-  rec( 1 := 11, 10 := 15, 2 := 12, 3 := 3, 4 := 4, 5 := 13, 6 := 6, 7 := 14, 
-      8 := 8, 9 := 9 ) ]
+  Transformation( [ 11, 12, 3, 4, 13, 6, 14, 8, 9, 15, 11, 12, 13, 14, 15 ] ) 
+ ]
 
 # AmalgamDigraphsIsomorphic
 gap> D1 := PetersenGraph();;
@@ -2789,7 +2789,7 @@ gap> D2 := Digraph([
 gap> U := AmalgamDigraphsIsomorphic(D1, D2, [3, 4, 6, 8, 9],
 >   [2, 4, 5, 6, 7]);
 [ <immutable digraph with 13 vertices, 42 edges>, 
-  rec( 1 := 11, 2 := 3, 3 := 12, 4 := 4, 5 := 8, 6 := 9, 7 := 6, 8 := 13 ) ]
+  Transformation( [ 11, 3, 12, 4, 8, 9, 6, 13, 9, 10, 11, 12, 13 ] ) ]
 gap> U := AmalgamDigraphsIsomorphic(D1, D2, [3, 4, 10, 8, 9],
 >   [2, 4, 5, 6, 7]);
 Error, the subdigraph induced by the 3rd argument (a list) in the 1st argument\