Add grouping algorithm

This might not be fully correct, but it seems to be accurate so far. Of
particular note, the vertex order needs to be deterministic for this
algorithm, which isn't provided by a map, since golang intentionally
randomizes it. As a result, this also adds a sorted version of
GetVertices called GetVerticesSorted.

config.go

@@ -336,8 +336,8 @@ func (ag *baseGrouper) init(g *Graph) error {
 	if ag.graph != nil {
 		return fmt.Errorf("The init method has already been called!")
 	}
-	ag.graph = g                         // pointer
-	ag.vertices = ag.graph.GetVertices() // cache
+	ag.graph = g                               // pointer
+	ag.vertices = ag.graph.GetVerticesSorted() // cache in deterministic order!
 	ag.i = 0
 	ag.j = 0
 	if len(ag.vertices) == 0 { // empty graph
@@ -437,23 +437,43 @@ func (ag *baseGrouper) vertexTest(b bool) (bool, error) {
 	return true, nil
 }
 
-type algorithmNameGrouper struct { // XXX rename me!
+// TODO: this algorithm may not be correct in all cases. replace if needed!
+type nonReachabilityGrouper struct {
 	baseGrouper // "inherit" what we want, and reimplement the rest
 }
 
-func (ag *algorithmNameGrouper) name() string {
-	log.Fatal("Not implemented!") // XXX
-	return "algorithmNameGrouper"
+func (ag *nonReachabilityGrouper) name() string {
+	return "nonReachabilityGrouper"
 }
 
-func (ag *algorithmNameGrouper) vertexNext() (v1, v2 *Vertex, err error) {
-	log.Fatal("Not implemented!") // XXX
-	// NOTE: you can even build this like this:
-	//v1, v2, err = ag.baseGrouper.vertexNext() // get all iterable pairs
-	// ...
-	//ag.baseGrouper.vertexTest(...)
-	//return
-	return nil, nil, fmt.Errorf("Not implemented!")
+// this algorithm relies on the observation that if there's a path from a to b,
+// then they *can't* be merged (b/c of the existing dependency) so therefore we
+// merge anything that *doesn't* satisfy this condition or that of the reverse!
+func (ag *nonReachabilityGrouper) vertexNext() (v1, v2 *Vertex, err error) {
+	for {
+		v1, v2, err = ag.baseGrouper.vertexNext() // get all iterable pairs
+		if err != nil {
+			log.Fatalf("Error running autoGroup(vertexNext): %v", err)
+		}
+
+		if v1 != v2 { // ignore self cmp early (perf optimization)
+			// if NOT reachable, they're viable...
+			out1 := ag.graph.Reachability(v1, v2)
+			out2 := ag.graph.Reachability(v2, v1)
+			if len(out1) == 0 && len(out2) == 0 {
+				return // return v1 and v2, they're viable
+			}
+		}
+
+		// if we got here, it means we're skipping over this candidate!
+		if ok, err := ag.baseGrouper.vertexTest(false); err != nil {
+			log.Fatalf("Error running autoGroup(vertexTest): %v", err)
+		} else if !ok {
+			return nil, nil, nil // done!
+		}
+
+		// the vertexTest passed, so loop and try with a new pair...
+	}
 }
 
 // autoGroup is the mechanical auto group "runner" that runs the interface spec
@@ -477,7 +497,9 @@ func (g *Graph) autoGroup(ag AutoGrouper) chan string {
 			wStr := fmt.Sprintf("%s", w)
 
 			if err := ag.vertexCmp(v, w); err != nil { // cmp ?
-				strch <- fmt.Sprintf("Compile: Grouping: !GroupCmp for: %s into %s", wStr, vStr)
+				if DEBUG {
+					strch <- fmt.Sprintf("Compile: Grouping: !GroupCmp for: %s into %s", wStr, vStr)
+				}
 
 				// remove grouped vertex and merge edges (res is safe)
 			} else if err := g.VertexMerge(v, w, ag.vertexMerge, ag.edgeMerge); err != nil { // merge...
@@ -506,7 +528,8 @@ func (g *Graph) autoGroup(ag AutoGrouper) chan string {
 func (g *Graph) AutoGroup() {
 	// receive log messages from channel...
 	// this allows test cases to avoid printing them when they're unwanted!
-	for str := range g.autoGroup(&baseGrouper{}) {
+	// TODO: this algorithm may not be correct in all cases. replace if needed!
+	for str := range g.autoGroup(&nonReachabilityGrouper{}) {
 		log.Println(str)
 	}
 }

main.go

@@ -155,7 +155,7 @@ func run(c *cli.Context) {
 			G = fullGraph.Copy() // copy to active graph
 			// XXX: do etcd transaction out here...
 			G.AutoEdges() // add autoedges; modifies the graph
-			//G.AutoGroup() // run autogroup; modifies the graph // TODO
+			G.AutoGroup() // run autogroup; modifies the graph
 			// TODO: do we want to do a transitive reduction?
 
 			log.Printf("Graph: %v", G) // show graph

pgraph.go

@@ -25,6 +25,7 @@ import (
 	"log"
 	"os"
 	"os/exec"
+	"sort"
 	"strconv"
 	"sync"
 	"syscall"
@@ -183,7 +184,8 @@ func (g *Graph) NumEdges() int {
 	return count
 }
 
-// get an array (slice) of all vertices in the graph
+// GetVertices returns a randomly sorted slice of all vertices in the graph
+// The order is random, because the map implementation is intentionally so!
 func (g *Graph) GetVertices() []*Vertex {
 	var vertices []*Vertex
 	for k := range g.Adjacency {
@@ -204,6 +206,23 @@ func (g *Graph) GetVerticesChan() chan *Vertex {
 	return ch
 }
 
+type VertexSlice []*Vertex
+
+func (vs VertexSlice) Len() int           { return len(vs) }
+func (vs VertexSlice) Swap(i, j int)      { vs[i], vs[j] = vs[j], vs[i] }
+func (vs VertexSlice) Less(i, j int) bool { return vs[i].String() < vs[j].String() }
+
+// GetVerticesSorted returns a sorted slice of all vertices in the graph
+// The order is sorted by String() to avoid the non-determinism in the map type
+func (g *Graph) GetVerticesSorted() []*Vertex {
+	var vertices []*Vertex
+	for k := range g.Adjacency {
+		vertices = append(vertices, k)
+	}
+	sort.Sort(VertexSlice(vertices)) // add determinism
+	return vertices
+}
+
 // make the graph pretty print
 func (g *Graph) String() string {
 	return fmt.Sprintf("Vertices(%d), Edges(%d)", g.NumVertices(), g.NumEdges())
@@ -546,22 +565,54 @@ func (g *Graph) VertexMerge(v1, v2 *Vertex, vertexMergeFn func(*Vertex, *Vertex)
 	// 2) edges that point towards v2 from X now point to v1 from X (no dupes)
 	for _, x := range g.IncomingGraphEdges(v2) { // all to vertex v (??? -> v)
 		e := g.Adjacency[x][v2] // previous edge
+		r := g.Reachability(x, v1)
 		// merge e with ex := g.Adjacency[x][v1] if it exists!
-		if ex, exists := g.Adjacency[x][v1]; exists && edgeMergeFn != nil {
+		if ex, exists := g.Adjacency[x][v1]; exists && edgeMergeFn != nil && len(r) == 0 {
 			e = edgeMergeFn(e, ex)
 		}
-		g.AddEdge(x, v1, e)        // overwrite edge
+		if len(r) == 0 { // if not reachable, add it
+			g.AddEdge(x, v1, e) // overwrite edge
+		} else if edgeMergeFn != nil { // reachable, merge e through...
+			prev := x // initial condition
+			for i, next := range r {
+				if i == 0 {
+					// next == prev, therefore skip
+					continue
+				}
+				// this edge is from: prev, to: next
+				ex, _ := g.Adjacency[prev][next] // get
+				ex = edgeMergeFn(ex, e)
+				g.Adjacency[prev][next] = ex // set
+				prev = next
+			}
+		}
 		delete(g.Adjacency[x], v2) // delete old edge
 	}
 
 	// 3) edges that point from v2 to X now point from v1 to X (no dupes)
 	for _, x := range g.OutgoingGraphEdges(v2) { // all from vertex v (v -> ???)
 		e := g.Adjacency[v2][x] // previous edge
+		r := g.Reachability(v1, x)
 		// merge e with ex := g.Adjacency[v1][x] if it exists!
-		if ex, exists := g.Adjacency[v1][x]; exists && edgeMergeFn != nil {
+		if ex, exists := g.Adjacency[v1][x]; exists && edgeMergeFn != nil && len(r) == 0 {
 			e = edgeMergeFn(e, ex)
 		}
-		g.AddEdge(v1, x, e) // overwrite edge
+		if len(r) == 0 {
+			g.AddEdge(v1, x, e) // overwrite edge
+		} else if edgeMergeFn != nil { // reachable, merge e through...
+			prev := v1 // initial condition
+			for i, next := range r {
+				if i == 0 {
+					// next == prev, therefore skip
+					continue
+				}
+				// this edge is from: prev, to: next
+				ex, _ := g.Adjacency[prev][next]
+				ex = edgeMergeFn(ex, e)
+				g.Adjacency[prev][next] = ex
+				prev = next
+			}
+		}
 		delete(g.Adjacency[v2], x)
 	}
 

pgraph_test.go

@@ -624,7 +624,7 @@ func (obj *NoopResTest) GroupCmp(r Res) bool {
 		return false
 	}
 
-	// TODO: implement this in vertexCmp for *testBaseGrouper instead?
+	// TODO: implement this in vertexCmp for *testGrouper instead?
 	if strings.Contains(res.Name, ",") { // HACK
 		return false // element to be grouped is already grouped!
 	}
@@ -755,15 +755,16 @@ Loop:
 	return nil // success!
 }
 
-type testBaseGrouper struct { // FIXME: update me when we've implemented the correct grouping algorithm!
-	baseGrouper // "inherit" what we want, and reimplement the rest
+type testGrouper struct {
+	// TODO: this algorithm may not be correct in all cases. replace if needed!
+	nonReachabilityGrouper // "inherit" what we want, and reimplement the rest
 }
 
-func (ag *testBaseGrouper) name() string {
-	return "testBaseGrouper"
+func (ag *testGrouper) name() string {
+	return "testGrouper"
 }
 
-func (ag *testBaseGrouper) vertexMerge(v1, v2 *Vertex) (v *Vertex, err error) {
+func (ag *testGrouper) vertexMerge(v1, v2 *Vertex) (v *Vertex, err error) {
 	if err := v1.Res.GroupRes(v2.Res); err != nil { // group them first
 		return nil, err
 	}
@@ -779,7 +780,7 @@ func (ag *testBaseGrouper) vertexMerge(v1, v2 *Vertex) (v *Vertex, err error) {
 	return // success or fail, and no need to merge the actual vertices!
 }
 
-func (ag *testBaseGrouper) edgeMerge(e1, e2 *Edge) *Edge {
+func (ag *testGrouper) edgeMerge(e1, e2 *Edge) *Edge {
 	// HACK: update the name so it makes a union of both names
 	n1 := strings.Split(e1.Name, ",") // load
 	n2 := strings.Split(e2.Name, ",") // load
@@ -805,9 +806,8 @@ func (g *Graph) fullPrint() (str string) {
 
 // helper function
 func runGraphCmp(t *testing.T, g1, g2 *Graph) {
-	// FIXME: update me when we've implemented the correct grouping algorithm!
-	ch := g1.autoGroup(&testBaseGrouper{}) // edits the graph
-	for _ = range ch {                     // bleed the channel or it won't run :(
+	ch := g1.autoGroup(&testGrouper{}) // edits the graph
+	for _ = range ch {                 // bleed the channel or it won't run :(
 		// pass
 	}
 	err := GraphCmp(g1, g2)
@@ -1126,13 +1126,14 @@ func TestPgraphGrouping15(t *testing.T) {
 	runGraphCmp(t, g1, g2)
 }
 
-/* FIXME: uncomment me when we've implemented the correct grouping algorithm!
-// reattach 1 (outer)
-// a1    a2         a1,a2
-//  |   /             |
-// b1  /      >>>    b1     (arrows point downwards)
-//  | /               |
-// c1                c1
+// re-attach 1 (outer)
+// technically the second possibility is valid too, depending on which order we
+// merge edges in, and if we don't filter out any unnecessary edges afterwards!
+// a1    a2         a1,a2        a1,a2
+//  |   /             |            |  \
+// b1  /      >>>    b1     OR    b1  /   (arrows point downwards)
+//  | /               |            | /
+// c1                c1           c1
 func TestPgraphGrouping16(t *testing.T) {
 	g1 := NewGraph("g1") // original graph
 	{
@@ -1153,14 +1154,14 @@ func TestPgraphGrouping16(t *testing.T) {
 		b1 := NewVertex(NewNoopResTest("b1"))
 		c1 := NewVertex(NewNoopResTest("c1"))
 		e1 := NewEdge("e1,e3")
-		e2 := NewEdge("e2") // TODO: should this be e2,e3 (eg we split e3?)
+		e2 := NewEdge("e2,e3") // e3 gets "merged through" to BOTH edges!
 		g2.AddEdge(a, b1, e1)
 		g2.AddEdge(b1, c1, e2)
 	}
 	runGraphCmp(t, g1, g2)
 }
 
-// reattach 2 (inner)
+// re-attach 2 (inner)
 // a1    b2          a1
 //  |   /             |
 // b1  /      >>>   b1,b2   (arrows point downwards)
@@ -1194,6 +1195,7 @@ func TestPgraphGrouping17(t *testing.T) {
 }
 
 // re-attach 3 (double)
+// similar to "re-attach 1", technically there is a second possibility for this
 // a2   a1    b2         a1,a2
 //   \   |   /             |
 //    \ b1  /      >>>   b1,b2   (arrows point downwards)
@@ -1222,18 +1224,18 @@ func TestPgraphGrouping18(t *testing.T) {
 		b := NewVertex(NewNoopResTest("b1,b2"))
 		c1 := NewVertex(NewNoopResTest("c1"))
 		e1 := NewEdge("e1,e3")
-		e2 := NewEdge("e2,e4")
+		e2 := NewEdge("e2,e3,e4") // e3 gets "merged through" to BOTH edges!
 		g2.AddEdge(a, b, e1)
 		g2.AddEdge(b, c1, e2)
 	}
 	runGraphCmp(t, g1, g2)
 }
 
-// tricky merge, (no change or merge?)
+// connected merge 0, (no change!)
 // a1            a1
 //   \     >>>     \     (arrows point downwards)
 //    a2            a2
-func TestPgraphGroupingTricky1(t *testing.T) {
+func TestPgraphGroupingConnected0(t *testing.T) {
 	g1 := NewGraph("g1") // original graph
 	{
 		a1 := NewVertex(NewNoopResTest("a1"))
@@ -1248,11 +1250,35 @@ func TestPgraphGroupingTricky1(t *testing.T) {
 		e1 := NewEdge("e1")
 		g2.AddEdge(a1, a2, e1)
 	}
-	//g3 := NewGraph("g2") // expected result ?
-	//{
-	//	a := NewVertex(NewNoopResTest("a1,a2"))
-	//}
-	runGraphCmp(t, g1, g2) // TODO: i'm tempted to think this is correct
-	//runGraphCmp(t, g1, g3)
+	runGraphCmp(t, g1, g2)
+}
+
+// connected merge 1, (no change!)
+// a1              a1
+//   \               \
+//    b      >>>      b      (arrows point downwards)
+//     \               \
+//      a2              a2
+func TestPgraphGroupingConnected1(t *testing.T) {
+	g1 := NewGraph("g1") // original graph
+	{
+		a1 := NewVertex(NewNoopResTest("a1"))
+		b := NewVertex(NewNoopResTest("b"))
+		a2 := NewVertex(NewNoopResTest("a2"))
+		e1 := NewEdge("e1")
+		e2 := NewEdge("e2")
+		g1.AddEdge(a1, b, e1)
+		g1.AddEdge(b, a2, e2)
+	}
+	g2 := NewGraph("g2") // expected result ?
+	{
+		a1 := NewVertex(NewNoopResTest("a1"))
+		b := NewVertex(NewNoopResTest("b"))
+		a2 := NewVertex(NewNoopResTest("a2"))
+		e1 := NewEdge("e1")
+		e2 := NewEdge("e2")
+		g2.AddEdge(a1, b, e1)
+		g2.AddEdge(b, a2, e2)
+	}
+	runGraphCmp(t, g1, g2)
 }
-*/