4

This is the code for dfs.

bool processed[MAXV+1]; /* which vertices have been processed */
bool discovered[MAXV+1]; /* which vertices have been found */
int parent[MAXV+1]; /* discovery relation */  
#define MAXV 1000 /* maximum number of vertices */

typedef struct {
int y;                   /* adjacency info */
int weight;             /* edge weight, if any */
struct edgenode *next; /* next edge in list */
} edgenode;

typedef struct {
edgenode *edges[MAXV+1]; /* adjacency info */
int degree[MAXV+1];     /* outdegree of each vertex */
int nvertices;         /* number of vertices in graph */
int nedges;            /* number of edges in graph */
bool directed;        /* is the graph directed? */
} graph;

dfs(graph *g, int v)
{

   edgenode *p;           /* temporary pointer */
   int y;                /* successor vertex */
   if (finished) return; /* allow for search termination */
   discovered[v] = TRUE;
   time = time + 1;
   entry_time[v] = time;
   process_vertex_early(v);
   p = g->edges[v];
   while (p != NULL) {
         y = p->y;
         if (discovered[y] == FALSE) 
         {
             parent[y] = v;
             process_edge(v,y);
             dfs(g,y);
         }
         else if ((!processed[y]) || (g->directed))
         process_edge(v,y);
         if (finished) return;

       p = p->next;

}
   process_vertex_late(v);
   time = time + 1;
   exit_time[v] = time;
   processed[v] = TRUE;
}

And for finding the cycles it has modified the process edge function like below

process_edge(int x, int y)
{
    if (parent[x] != y) { /* found back edge! */
       printf("Cycle from %d to %d:",y,x);
    find_path(y,x,parent);
    printf("\n\n");
    finished = TRUE;
    }
}

Now imagine a small tree with just 2 leaf nodes and one root. When this tree is subjected to this function, I believe it will say that it has cycles. which is wrong !! Please correct me if i am wrong. Thanks.

jairaj
  • 1,789
  • 5
  • 19
  • 32

4 Answers4

7

From the errata corrige, at http://www.cs.sunysb.edu/~skiena/algorist/book/errata:

(*) Page 173, process_edge procedure -- the correct test should be

if (discovered[y] && (parent[x] != y)) { /* found back edge */

Antonio Barbuzzi
  • 646
  • 7
  • 11
1

I think you're right, and the code is wrong.

It looks to me like the problem is if (parent[x] != y) in process_edge(). In both calls to process_edge(), the supposed parent is passed before the supposed child, i.e. inside process_edge() we expect x to be the parent, so I think that line should be if (parent[y] != x).

j_random_hacker
  • 50,331
  • 10
  • 105
  • 169
  • exactly!! that is what i thought. But i just want to be sure. – jairaj Dec 15 '12 at 03:59
  • Well, fixing it would certainly help. I'm not quite up to proving correctness of the rest of the algorithm, especially since I don't know what `edgenode` contains, how `discovered` and `visited` are initialised, what `find_path()` does, what representation is used for edges etc. – j_random_hacker Dec 15 '12 at 04:04
  • Why not just run the changed code on a few examples, and see how it does first? – j_random_hacker Dec 15 '12 at 04:13
  • 2
    With the change from the errata, the find cycle code is correct because it's saying "Of the vertex this edge points to, if it's undiscovered, that's ok, we're proceeding down the DFS. If it's my parent, we're on an undirected graph, and that's ok. But if it's discovered and not my parent, it must be a back edge, and hence a cycle." The trick is to realize this test is only relevant after you've already processed the parent. – kball Jul 15 '13 at 08:31
0

Sadly, I think this dfs code is just wrong. It's been a long time since I've studied this stuff in detail, but I think it's clear that the code simply doesn't do what he says it does.

The find cycle code is correct (with the change shown in the errata, as noted by Antonio).

The main problem is that the find cycle routine is in process_edge, but he doesn't process edges to previously discovered nodes! So how will he find the cycle?! If you're interested in cycles (or back edges for any reason), I should think you MUST process all edges.

If you're not interested in back edges and want to avoid processing them, then the code as presented is correct.

It's brilliantly ironic that in the passage immediately preceding the Finding Cycles section in the text he writes:

I find that the subtlety of depth-first search-based algorithms kicks me in the head whenever I try to implement one.

You don't say! :P

The while loop should look something like this:

...

while (p != NULL) {
    y = p.y;

    process_edge(v,y);

    if (discovered[y] == FALSE) {
        parent[y] = v;
        dfs(g,y);
    }

    if (finished) {
        return;
    }

    p = p.next;
}

...
kball
  • 4,923
  • 3
  • 29
  • 31
  • Actually the code in the book does process previously discovered nodes as long as the node hasn't been processed. – Mike Jan 21 '16 at 02:46
0

In undirected graphs there are only tree and back edges (no forward or cross edges) . The sufficient condition it to be a tree edge is 

if (discovered[y] == FALSE)

But in case of the vertices which have been discovered there is a chance that the undirected edge goes back to its parent (instead of its ancestor) . To prevent this case , another condition is added :

if (discovered[y]==TRUE && (parent[x] != y))

For example : y has been discovered by x (say).When the recursive algorithm moves to vertex y (child of x) , in that case vertex X is now vertex y and vertex Y is parent(y) (or x!). If the second condition is removed there is a possibility that the algorithm detects the edge going from child(y) to its parent(x) (in an undirected graph ) as an edge going back to one of the former ancestors which is completely wrong .

saksham jindal
  • 147
  • 5