diff --git a/config.go b/config.go index fe42b785..87f9e35f 100644 --- a/config.go +++ b/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) } } diff --git a/main.go b/main.go index ec9eb03d..079673b9 100644 --- a/main.go +++ b/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 diff --git a/pgraph.go b/pgraph.go index a19850bb..39805d53 100644 --- a/pgraph.go +++ b/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) } diff --git a/pgraph_test.go b/pgraph_test.go index f9722e28..3cfa0085 100644 --- a/pgraph_test.go +++ b/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) } -*/