Skip to content
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

ExecuteDFS (generic depth first search) #459

Merged
merged 65 commits into from
May 26, 2021

Conversation

LRacine
Copy link
Contributor

@LRacine LRacine commented Apr 29, 2021

In this pull request, I submit my work for VP2215.
I did this to resolve the issue noted in #393

From this pull request you will see that I have created:

  • A generic depth first search algorithm ExecuteDFS that takes a record, data object, start vertex,
    preorder function, postorder function, ancestor function and a cross function.
  • NewDFSRecord which creates a record for the ExecuteDFS function that stores the preorder, postorders, parent,
    current and child attributes
  • DFSDefault which acts as a default argument for the ExecuteDFS function

The generic depth first search algorithm works as follows:

While there are nodes in our stack
  pop node from stack
   If we are backtracking on this node
     PostOrderFunc()
     Set and save the post order number for the node
   elif we’ve visited it but we are not backtracking
     Continue;
   else
     Set and save the pre order number for the node
     Add it back to the stack for backtracking
 
   Return if the user want to

   Go through the node’s neighbours
     if the neighbour is unvisited
       Set its parent as the node
       Add it to the stack
     elif we haven’t backtracked on the neighbour
        AncestorFunc()
     elif we visited the neighbour before the node
       CrossFunc()
     Return if the user want to

I also refactored the existing DFS implementations in:

  • UndirectedSpanningForest
  • DigraphPath
  • DigraphTopologicalSort
  • DigraphLongestDistanceFromVertex
  • IsAntisymmetricDigraph
  • IsAcyclicDigraph
  • DIGRAPHS_ArticulationPointsBridgesStrongOrientation
  • VerticesReachableFrom
  • DominatorTree

Copy link
Contributor

@marinaanagno marinaanagno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments on doc concerning theory etc :) @james-d-mitchell you have looked at this project closely so maybe reachability and connectivity etc were misinterpreted by me, but any views on this specific point about connected components etc would be most valuable :)

doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated
<A>record</A>.child) is an edge and <A>record</A>.child has been visited before <A>record</A>.current
and it is not an ancestor of <A>record</A>.current</Item>
</List>
Note that this function only performs a depth first search on the connected graph that has <A>start</A>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Note that this function only performs a depth first search on the connected graph that has <A>start</A>.
Note that this function only performs a depth first search on the connected component that contains <A>start</A>.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think however that this line should be completely changed to "the digraph induced by the vertices reachable from start.
In undirected graphs this would indeed be the connected component containing start. However in directed graphs we might have a connected graph and vertices that are not reachable from the start vertex :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is much clearer!

doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated
causing us to backtrack. This vertex is stored in <A>record</A>.child and its parent is stored
in <A>record</A>.current</Item>
<Item><A>AncestorFunc</A>: this function is called when (<A>record</A>.current,
<A>record</A>.child) is an edge and <A>record</A>.child is an ancestor of <A>record</A>.current. An ancestor here means that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<A>record</A>.child) is an edge and <A>record</A>.child is an ancestor of <A>record</A>.current. An ancestor here means that
<A>record</A>.child) is an edge and <C><A>record</A>.child</C> is an ancestor of <A>record</A>.current. An ancestor here means that

doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated

The record also stores a further 4 attributes.
<List>
<Item><E>current</E>: the current vertex that is being visited</Item>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use mark here too?

doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated
</List>

Initially, the current and child attributes will have -1 values and the lists (parent,
preorder and postorder) will have -1 values at all of their indicies as no vertex has
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
preorder and postorder) will have -1 values at all of their indicies as no vertex has
<C>preorder</C> and <C>postorder</C>) will have <C>-1</C> values at all of their indices as no vertex has

doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated Show resolved Hide resolved
doc/oper.xml Outdated Show resolved Hide resolved
Copy link
Member

@james-d-mitchell james-d-mitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor suggestions for the doc, I think this is great otherwise, and is a real improvement!

@LRacine
Copy link
Contributor Author

LRacine commented May 26, 2021

Extreme tests:

UndirectedSpanningForest

D := JohnsonDigraph(14, 7)
UndirectedSpanningForest(D);

DigraphPath

D := JohnsonDigraph(14, 7)
DigraphPath(D, 40, 1000);

DigraphTopologicalSort

gap> r := rec(DigraphNrVertices := 20000,
>             DigraphSource     := [],
>             DigraphRange      := []);;
gap> for i in [1 .. 9999] do
>   Add(r.DigraphSource, i);
>   Add(r.DigraphRange, i + 1);
> od;
> Add(r.DigraphSource, 10000);; Add(r.DigraphRange, 20000);;
> Add(r.DigraphSource, 10001);; Add(r.DigraphRange, 1);;
> for i in [10001 .. 19999] do
>   Add(r.DigraphSource, i);
>   Add(r.DigraphRange, i + 1);
> od;
gap> circuit := Digraph(r);
<immutable digraph with 20000 vertices, 20000 edges>
gap> topo := DigraphTopologicalSort(circuit);;

DigraphLongestDistanceFromVertex

D := ChainDigraph(10000);
DigraphLongestDistanceFromVertex(D, 2);

IsAntisymmetricDigraph

D := ChainDigraph(100000);
IsAntisymmetricDigraph(D);

IsAcyclicDigraph

D := JohnsonDigraph(14, 7);
IsAcyclicDigraph(D);

DIGRAPHS_ArticulationPointsBridgesStrongOrientation

D := JohnsonDigraph(14, 7);
DIGRAPHS_ArticulationPointsBridgesStrongOrientation(D);

VerticesReachableFrom

D := JohnsonDigraph(14, 7);
VerticesReachableFrom(D, 2);

DominatorTree

D := JohnsonDigraph(14, 7);
DominatorTree(D,2);

@LRacine LRacine merged commit 8e9d7de into digraphs:master May 26, 2021
wilfwilson added a commit to wilfwilson/Digraphs that referenced this pull request Oct 27, 2021
In particular, this pretty much reverses commit:
d9f060c
wilfwilson added a commit to wilfwilson/Digraphs that referenced this pull request Oct 27, 2021
In particular, this pretty much reverses commit:
d9f060c
wilfwilson added a commit that referenced this pull request Oct 27, 2021
In particular, this pretty much reverses commit:
d9f060c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A label for PRs that provide enhancements. new-feature A label for new features.
Projects
None yet
5 participants