dfs: attempt to fix

Author: james-d-mitchell

gap/attr.gi

@@ -702,7 +702,7 @@ function(D)
     data.out[data.count] := record.child;
   end;
   for i in DigraphVertices(D) do
-    if record.preorder[i] <> -1 then
+    if not IsBound(record.preorder[i]) then
       continue;
     fi;
     ExecuteDFS(record, data, i, DFSDefault,

gap/oper.gi

@@ -1388,10 +1388,10 @@ end);
 InstallMethod(DigraphPath, "for a digraph by out-neighbours and two pos ints",
 [IsDigraphByOutNeighboursRep, IsPosInt, IsPosInt],
 function(D, u, v)
-  local verts, record, out, path_info, PostOrderFunc, AncestorFunc, AddToPath;
+  local N, record, PreOrderFunc, AncestorFunc, nodes, edges, current;
 
-  verts := DigraphVertices(D);
-  if not (u in verts and v in verts) then
+  N := DigraphNrVertices(D);
+  if u > N or v > N then
     ErrorNoReturn("the 2nd and 3rd arguments <u> and <v> must be ",
                   "vertices of the 1st argument <D>,");
   elif IsDigraphEdge(D, u, v) then
@@ -1404,39 +1404,58 @@ function(D, u, v)
       and DigraphConnectedComponents(D).id[u] <>
           DigraphConnectedComponents(D).id[v] then
     return fail;
+  elif OutDegreeOfVertex(D, u) = 0 
+      or (HasInNeighbours(D) and InDegreeOfVertex(D, v) = 0) then
+    return fail;
   fi;
   record := NewDFSRecord(D);
-  path_info := Stack();
-  AddToPath := function(current, child)
-    local edge;
-    edge := Position(OutNeighborsOfVertex(D, current), child);
-    Push(path_info, edge);
-    Push(path_info, child);
-  end;
-  AncestorFunc := function(record, data)
-    if u = v and record.child = u and Size(data) = 0 then
-      AddToPath(record.current, record.child);
-      record.preorder[v] := DigraphNrVertices(D) + 1;
-    fi;
-  end;
-  PostOrderFunc := function(record, data)
-    if record.child <> u and
-        record.preorder[record.child] <= record.preorder[v] then
-      AddToPath(record.current, record.child);
-    fi;
-  end;
-  ExecuteDFS(record, path_info, u,
-               DFSDefault, PostOrderFunc,
-               AncestorFunc, DFSDefault);
-  if Size(path_info) <= 1 then
+  if u <> v then
+    # if v is reachable from u, then u is an ancestor of v, and so at some
+    # point v will be encountered for the first time, and PreOrderFunc will be
+    # called. 
+    PreOrderFunc := function(record, data)
+      if record.current = v then
+        record.stop := true;
+      fi;
+    end;
+    AncestorFunc := DFSDefault;
+  else
+    # if u is reachable from u, then u will be encountered as an ancestor of
+    # itself, but PreOrderFunc won't be called (because u has already been
+    # discovered). 
+    PreOrderFunc := DFSDefault;
+    AncestorFunc := function(record, data)
+      if record.child = v then
+        record.stop := true;
+      fi;
+    end;
+  fi;
+  ExecuteDFS(record, 
+             fail, 
+             u,
+             PreOrderFunc, 
+             DFSDefault,
+             AncestorFunc, 
+             DFSDefault);
+  if not record.stop then
     return fail;
   fi;
-  out := [[u], []];
-  while Size(path_info) <> 0 do
-    Add(out[1], Pop(path_info));
-    Add(out[2], Pop(path_info));
+  nodes := [v];
+  edges := [];
+  current := v;
+  if u = v then
+    # Go one back from v to the last node in the tree
+    current := record.current;
+    Add(nodes, current);
+    Add(edges, Position(OutNeighboursOfVertex(D, current), u));
+  fi;
+  # Follow the path from current (which is a descendant of u) back to u
+  while current <> u do
+    Add(edges, record.edge[current]);
+    current := record.parent[current];
+    Add(nodes, current);
   od;
-  return out;
+  return [Reversed(nodes), Reversed(edges)];
 end);
 
 InstallMethod(IsDigraphPath, "for a digraph and list",
@@ -2042,12 +2061,12 @@ function(D, root)
   end;
 
   ExecuteDFS(record,
-               data,
-               root,
-               PreOrderFunc,
-               DFSDefault,
-               AncestorFunc,
-               DFSDefault);
+             data,
+             root,
+             PreOrderFunc,
+             DFSDefault,
+             AncestorFunc,
+             DFSDefault);
   return data.result;
 end);
 
@@ -2254,9 +2273,10 @@ function(graph)
   record.child := -1;
   record.current := -1;
   record.stop := false;
-  record.parent := ListWithIdenticalEntries(DigraphNrVertices(graph), -1);
-  record.preorder := ListWithIdenticalEntries(DigraphNrVertices(graph), -1);
-  record.postorder := ListWithIdenticalEntries(DigraphNrVertices(graph), -1);
+  record.parent := [fail];
+  record.preorder := [fail];
+  record.postorder := [fail];
+  record.edge := [fail];
   return record;
 end);
 
@@ -2279,8 +2299,9 @@ end);
 InstallGlobalFunction(ExecuteDFS,
 function(record, data, start, PreOrderFunc, PostOrderFunc, AncestorFunc,
          CrossFunc)
-  if not IsEqualSet(RecNames(record), ["stop", "graph", "child", "parent",
-                                    "preorder", "postorder", "current"]) then
+  if not IsEqualSet(RecNames(record),
+                    ["stop", "graph", "child", "parent", "preorder",
+                     "postorder", "current", "edge"]) then
     ErrorNoReturn("the 1st argument <record> must be created with ",
                   "NewDFSRecord,");
   fi;

src/dfs.c

@@ -32,16 +32,16 @@ Obj ExecuteDFS(Obj self, Obj args) {
   Obj AncestorFunc  = ELM_PLIST(args, 6);
   Obj CrossFunc     = ELM_PLIST(args, 7);
 
-  DIGRAPHS_ASSERT(NARG_FUNC(PreorderFunc) == 2);
-  DIGRAPHS_ASSERT(IS_FUNC(AncestorFunc));
-  DIGRAPHS_ASSERT(NARG_FUNC(AncestorFunc) == 2);
-  DIGRAPHS_ASSERT(IS_FUNC(PostOrderFunc));
-  DIGRAPHS_ASSERT(NARG_FUNC(PostOrderFunc) == 2);
-  DIGRAPHS_ASSERT(IS_FUNC(PreorderFunc));
   DIGRAPHS_ASSERT(IS_PREC(record));
   DIGRAPHS_ASSERT(IS_INTOBJ(start));
+  // DIGRAPHS_ASSERT(NARG_FUNC(PreorderFunc) == 2);
+  DIGRAPHS_ASSERT(IS_FUNC(PreorderFunc));
+  DIGRAPHS_ASSERT(IS_FUNC(PostOrderFunc));
+  // DIGRAPHS_ASSERT(NARG_FUNC(PostOrderFunc) == 2);
+  DIGRAPHS_ASSERT(IS_FUNC(AncestorFunc));
+  // DIGRAPHS_ASSERT(NARG_FUNC(AncestorFunc) == 2);
   DIGRAPHS_ASSERT(IS_FUNC(CrossFunc));
-  DIGRAPHS_ASSERT(NARG_FUNC(CrossFunc) == 2);
+  // DIGRAPHS_ASSERT(NARG_FUNC(CrossFunc) == 2);
 
   Obj D = ElmPRec(record, RNamName("graph"));
   Int N = DigraphNrVertices(D);
@@ -51,7 +51,7 @@ Obj ExecuteDFS(Obj self, Obj args) {
         "the third argument <start> must be a vertex in your graph,", 0L, 0L);
   }
   Int top   = 0;
-  Obj stack = NEW_PLIST(T_PLIST_CYC, N);
+  Obj stack = NEW_PLIST(T_PLIST_CYC, 0);
   AssPlist(stack, ++top, start);
 
   UInt preorder_num  = 0;
@@ -62,12 +62,14 @@ Obj ExecuteDFS(Obj self, Obj args) {
   Obj parent    = ElmPRec(record, RNamName("parent"));
   Obj postorder = ElmPRec(record, RNamName("postorder"));
   Obj preorder  = ElmPRec(record, RNamName("preorder"));
+  Obj edge      = ElmPRec(record, RNamName("edge"));
 
-  DIGRAPHS_ASSERT(LEN_PLIST(parent) == N);
-  DIGRAPHS_ASSERT(LEN_PLIST(postorder) == N);
-  DIGRAPHS_ASSERT(LEN_PLIST(preorder) == N);
+  DIGRAPHS_ASSERT(LEN_PLIST(parent) == 1);
+  DIGRAPHS_ASSERT(LEN_PLIST(postorder) == 1);
+  DIGRAPHS_ASSERT(LEN_PLIST(preorder) == 1);
+  DIGRAPHS_ASSERT(LEN_PLIST(edge) == 1);
 
-  SET_ELM_PLIST(parent, INT_INTOBJ(start), start);
+  AssPlist(parent, INT_INTOBJ(start), start);
 
   Obj neighbors = FuncOutNeighbours(self, D);
   DIGRAPHS_ASSERT(IS_PLIST(neighbors));
@@ -77,22 +79,27 @@ Obj ExecuteDFS(Obj self, Obj args) {
   Int RNamStop    = RNamName("stop");
 
   while (top > 0) {
+    if (ElmPRec(record, RNamStop) == True) {
+      break;
+    }
     current = INT_INTOBJ(ELM_PLIST(stack, top--));
     DIGRAPHS_ASSERT(current != 0);
     if (current < 0) {
       Int child = -1 * current;
       AssPRec(record, RNamChild, INTOBJ_INT(child));
       AssPRec(record, RNamCurrent, ELM_PLIST(parent, child));
       CALL_2ARGS(PostOrderFunc, record, data);
-      SET_ELM_PLIST(postorder, child, INTOBJ_INT(++postorder_num));
+      AssPlist(postorder, child, INTOBJ_INT(++postorder_num));
       CHANGED_BAG(record);
       continue;
-    } else if (INT_INTOBJ(ELM_PLIST(preorder, current)) > 0) {
+    } else if (current <= LEN_PLIST(preorder)
+               && ELM_PLIST(preorder, current) != 0
+               && ELM_PLIST(preorder, current) != Fail) {
       continue;
     } else {
       AssPRec(record, RNamCurrent, INTOBJ_INT(current));
       CALL_2ARGS(PreorderFunc, record, data);
-      SET_ELM_PLIST(preorder, current, INTOBJ_INT(++preorder_num));
+      AssPlist(preorder, current, INTOBJ_INT(++preorder_num));
       CHANGED_BAG(record);
       AssPlist(stack, ++top, INTOBJ_INT(-1 * current));
     }
@@ -105,11 +112,15 @@ Obj ExecuteDFS(Obj self, Obj args) {
     for (UInt j = 0; j < LEN_LIST(succ); ++j) {
       UInt v = INT_INTOBJ(ELM_LIST(succ, LEN_LIST(succ) - j));
       AssPRec(record, RNamChild, INTOBJ_INT(v));
-      if (INT_INTOBJ(ELM_PLIST(preorder, v)) == -1) {
-        SET_ELM_PLIST(parent, v, INTOBJ_INT(current));
+      if (v > LEN_PLIST(preorder) || ELM_PLIST(preorder, v) == 0
+          || ELM_PLIST(preorder, v) == Fail) {
+        AssPlist(parent, v, INTOBJ_INT(current));
+        AssPlist(edge, v, INTOBJ_INT(LEN_LIST(succ) - j));
         CHANGED_BAG(record);
         AssPlist(stack, ++top, INTOBJ_INT(v));
-      } else if (INT_INTOBJ(ELM_PLIST(postorder, v)) == -1) {
+      } else if (current > LEN_PLIST(preorder)
+                 || ELM_PLIST(preorder, current) != 0
+                 || ELM_PLIST(preorder, current) != Fail) {
         CALL_2ARGS(AncestorFunc, record, data);
       } else if (INT_INTOBJ(ELM_PLIST(preorder, v))
                  < INT_INTOBJ(ELM_PLIST(preorder, current))) {