[p1-greedy] Properly initialize degree maps (fixes #9)

nulab · Oct 3, 2024 · 65d0a80 · 65d0a80
1 parent 81cd180
commit 65d0a80
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 7 deletions.
diff --git a/internal/phase1/greedy.go b/internal/phase1/greedy.go
@@ -36,10 +36,24 @@ func execGreedy(g *graph.DGraph, params graph.Params) {
 		indeg:   graph.NodeIntMap{},
 	}
 
-	nodeCount := len(g.Nodes)
+	var (
+		nodeCount = len(g.Nodes)
+		sources   []*graph.Node
+		sinks     []*graph.Node
+	)
 
-	sources := slices.Collect(g.Sources())
-	sinks := slices.Collect(g.Sinks())
+	for _, n := range g.Nodes {
+		p.indeg[n] = n.Indeg()
+		if n.Indeg() == 0 {
+			sources = append(sources, n)
+		}
+		p.outdeg[n] = n.Outdeg()
+		if n.Outdeg() == 0 {
+			sinks = append(sinks, n)
+		}
+	}
+	sources = slices.Clip(sources)
+	sinks = slices.Clip(sinks)
 
 	// here ELK accounts for edge priority: particular edges that the user doesn't want to reverse
 	// can be assigned a non-zero priority; this will artificially increase the node's in-/out-degrees.

diff --git a/internal/testfiles/adjacency_lists.go b/internal/testfiles/adjacency_lists.go
@@ -15,6 +15,30 @@ var issues1and4 = [][]string{
 	{"N2", "Nd"},
 }
 
+var issue9 = [][]string{
+	{"N1", "N4"},
+	{"N1", "N8"},
+	{"N2", "N5"},
+	{"N2", "N8"},
+	{"N3", "N8"},
+	{"N6", "N3"},
+	{"N8", "N1"},
+	{"N8", "N2"},
+	{"N8", "N7"},
+	{"N8", "N15"},
+	{"N8", "N16"},
+	{"N9", "N10"},
+	{"N10", "N11"},
+	{"N12", "N13"},
+	{"N13", "N14"},
+	{"N15", "N1"},
+	{"N15", "N9"},
+	{"N15", "N10"},
+	{"N16", "N2"},
+	{"N16", "N12"},
+	{"N16", "N13"},
+}
+
 var simpleVirtualNodes = [][]string{
 	{"N1", "N2"},
 	{"N2", "N3"},

diff --git a/internal/testfiles/bugfix_test.go b/internal/testfiles/bugfix_test.go
@@ -14,10 +14,14 @@ import (
 )
 
 func TestCrashers(t *testing.T) {
-	t.Run("phase4 SinkColoring", func(t *testing.T) {
-		t.Run("program hangs", func(t *testing.T) {
-			src := graph.EdgeSlice(issues1and4)
-			assert.NotPanics(t, func() { _ = autog.Layout(src, autog.WithPositioning(autog.PositioningSinkColoring)) })
+	t.Run("phase1", func(t *testing.T) {
+		t.Run("greedy cycle breaker fails to break cycles", func(t *testing.T) {
+			assert.NotPanics(t, func() {
+				_ = autog.Layout(
+					graph.EdgeSlice(issue9),
+					autog.WithNonDeterministicGreedyCycleBreaker(),
+				)
+			})
 		})
 	})
 
@@ -47,6 +51,13 @@ func TestCrashers(t *testing.T) {
 		})
 	})
 
+	t.Run("phase4 SinkColoring", func(t *testing.T) {
+		t.Run("program hangs", func(t *testing.T) {
+			src := graph.EdgeSlice(issues1and4)
+			assert.NotPanics(t, func() { _ = autog.Layout(src, autog.WithPositioning(autog.PositioningSinkColoring)) })
+		})
+	})
+
 	t.Run("phase4 NetworkSimplex", func(t *testing.T) {
 		g := &ig.DGraph{}
 		graph.EdgeSlice(DotAbstract).Populate(g)