graph: make graph analysis routines safe for indeterminate iterators

This is a change in design for the graph.NodesOf family of functions. The
alternative was to provide an equivalent set of non-panicking routines in
graph for internal use. The protection that was intended with the panic
was to panic early rather than late when an indeterminate iterator exhausts
slice index space. I think in hindsight this was an error and we should
let things blow up in that (likely rare) situation.

The majority of changes are in test code. Outside the iterator package, which
is intimately tied to the determined iterator implementations, only one test
now fails if an indeterminate iterator is used, product's Modular extended
sub-graph isomorphism example, which is an algorithm that would have time
complexity issues with large iterators anyway.

graph/complement_test.go

@@ -28,7 +28,7 @@ var complementTests = []struct {
 
 func TestComplement(t *testing.T) {
 	for _, test := range complementTests {
-		n := test.g.Nodes().Len()
+		n := len(graph.NodesOf(test.g.Nodes()))
 		wantM := n * (n - 1) // Double counting edges, but no self-loops.
 
 		var gotM int
@@ -72,6 +72,10 @@ var nodeFilterIteratorTests = []struct {
 func TestNodeFilterIterator(t *testing.T) {
 	for _, test := range nodeFilterIteratorTests {
 		it := graph.NewNodeFilterIterator(test.src, test.filter, test.root)
+		if it.Len() < 0 {
+			t.Logf("don't test indeterminate iterators: %T", it)
+			continue
+		}
 		for i := 0; i < 2; i++ {
 			n := it.Len()
 			if n != test.len {

graph/graphs/gen/batagelj_brandes_test.go

@@ -221,17 +221,16 @@ func TestPowerLawUndirected(t *testing.T) {
 				t.Fatalf("unexpected error: n=%d, d=%d: %v", n, d, err)
 			}
 
-			nodes := g.Nodes()
-			if nodes.Len() != n {
-				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, nodes.Len())
+			nodes := graph.NodesOf(g.Nodes())
+			if len(nodes) != n {
+				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, len(nodes))
 			}
 
-			for nodes.Next() {
-				u := nodes.Node()
+			for _, u := range nodes {
 				uid := u.ID()
 				var lines int
 				for _, v := range graph.NodesOf(g.From(uid)) {
-					lines += g.Lines(uid, v.ID()).Len()
+					lines += len(graph.LinesOf(g.Lines(uid, v.ID())))
 				}
 				if lines < d {
 					t.Errorf("unexpected degree below d: n=%d, d=%d: got:%d", n, d, lines)
@@ -252,17 +251,16 @@ func TestPowerLawDirected(t *testing.T) {
 				t.Fatalf("unexpected error: n=%d, d=%d: %v", n, d, err)
 			}
 
-			nodes := g.Nodes()
-			if nodes.Len() != n {
-				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, nodes.Len())
+			nodes := graph.NodesOf(g.Nodes())
+			if len(nodes) != n {
+				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, len(nodes))
 			}
 
-			for nodes.Next() {
-				u := nodes.Node()
+			for _, u := range nodes {
 				uid := u.ID()
 				var lines int
 				for _, v := range graph.NodesOf(g.From(uid)) {
-					lines += g.Lines(uid, v.ID()).Len()
+					lines += len(graph.LinesOf(g.Lines(uid, v.ID())))
 				}
 				if lines < d {
 					t.Errorf("unexpected degree below d: n=%d, d=%d: got:%d", n, d, lines)
@@ -283,9 +281,9 @@ func TestBipartitePowerLawUndirected(t *testing.T) {
 				t.Fatalf("unexpected error: n=%d, d=%d: %v", n, d, err)
 			}
 
-			nodes := g.Nodes()
-			if nodes.Len() != 2*n {
-				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, nodes.Len())
+			nodes := graph.NodesOf(g.Nodes())
+			if len(nodes) != 2*n {
+				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, len(nodes))
 			}
 			if len(p1) != n {
 				t.Errorf("unexpected number of nodes in p1: n=%d, d=%d: got:%d", n, d, len(p1))
@@ -307,12 +305,11 @@ func TestBipartitePowerLawUndirected(t *testing.T) {
 				t.Errorf("unexpected overlap in partition membership: n=%d, d=%d: got:%d", n, d, len(o))
 			}
 
-			for nodes.Next() {
-				u := nodes.Node()
+			for _, u := range nodes {
 				uid := u.ID()
 				var lines int
 				for _, v := range graph.NodesOf(g.From(uid)) {
-					lines += g.Lines(uid, v.ID()).Len()
+					lines += len(graph.LinesOf(g.Lines(uid, v.ID())))
 				}
 				if lines < d {
 					t.Errorf("unexpected degree below d: n=%d, d=%d: got:%d", n, d, lines)
@@ -333,9 +330,9 @@ func TestBipartitePowerLawDirected(t *testing.T) {
 				t.Fatalf("unexpected error: n=%d, d=%d: %v", n, d, err)
 			}
 
-			nodes := g.Nodes()
-			if nodes.Len() != 2*n {
-				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, nodes.Len())
+			nodes := graph.NodesOf(g.Nodes())
+			if len(nodes) != 2*n {
+				t.Errorf("unexpected number of nodes in graph: n=%d, d=%d: got:%d", n, d, len(nodes))
 			}
 			if len(p1) != n {
 				t.Errorf("unexpected number of nodes in p1: n=%d, d=%d: got:%d", n, d, len(p1))
@@ -357,12 +354,11 @@ func TestBipartitePowerLawDirected(t *testing.T) {
 				t.Errorf("unexpected overlap in partition membership: n=%d, d=%d: got:%d", n, d, len(o))
 			}
 
-			for nodes.Next() {
-				u := nodes.Node()
+			for _, u := range nodes {
 				uid := u.ID()
 				var lines int
 				for _, v := range graph.NodesOf(g.From(uid)) {
-					lines += g.Lines(uid, v.ID()).Len()
+					lines += len(graph.LinesOf(g.Lines(uid, v.ID())))
 				}
 				if lines < d {
 					t.Errorf("unexpected degree below d: n=%d, d=%d: got:%d", n, d, lines)

graph/layout/eades.go

@@ -65,13 +65,15 @@ func (u *EadesR2) Update(g graph.Graph, layout LayoutR2) bool {
 		}
 		u.nodes = g.Nodes()
 		u.indexOf = make(map[int64]int, u.nodes.Len())
-		u.particles = make([]barneshut.Particle2, 0, u.nodes.Len())
-		u.forces = make([]r2.Vec, u.nodes.Len())
+		if u.nodes.Len() >= 0 {
+			u.particles = make([]barneshut.Particle2, 0, u.nodes.Len())
+		}
 		for u.nodes.Next() {
 			id := u.nodes.Node().ID()
 			u.indexOf[id] = len(u.particles)
 			u.particles = append(u.particles, eadesR2Node{id: id, pos: r2.Vec{X: rnd(), Y: rnd()}})
 		}
+		u.forces = make([]r2.Vec, len(u.particles))
 	}
 	u.nodes.Reset()
 

graph/layout/plotter_test.go

@@ -27,8 +27,14 @@ func (p render) Plot(c draw.Canvas, plt *plot.Plot) {
 	if nodes.Len() == 0 {
 		return
 	}
-	xys := make(plotter.XYs, 0, nodes.Len())
-	ids := make([]string, 0, nodes.Len())
+	var (
+		xys plotter.XYs
+		ids []string
+	)
+	if nodes.Len() >= 0 {
+		xys = make(plotter.XYs, 0, nodes.Len())
+		ids = make([]string, 0, nodes.Len())
+	}
 	for nodes.Next() {
 		u := nodes.Node()
 		uid := u.ID()
@@ -81,7 +87,10 @@ func (p render) DataRange() (xmin, xmax, ymin, ymax float64) {
 	if nodes.Len() == 0 {
 		return
 	}
-	xys := make(plotter.XYs, 0, nodes.Len())
+	var xys plotter.XYs
+	if nodes.Len() >= 0 {
+		xys = make(plotter.XYs, 0, nodes.Len())
+	}
 	for nodes.Next() {
 		u := nodes.Node()
 		uid := u.ID()
@@ -98,12 +107,16 @@ func (p render) GlyphBoxes(plt *plot.Plot) []plot.GlyphBox {
 	if nodes.Len() == 0 {
 		return nil
 	}
-	b := make([]plot.GlyphBox, nodes.Len())
+	var b []plot.GlyphBox
+	if nodes.Len() >= 0 {
+		b = make([]plot.GlyphBox, 0, nodes.Len())
+	}
 	for i := 0; nodes.Next(); i++ {
 		u := nodes.Node()
 		uid := u.ID()
 		ur2 := p.GraphR2.LayoutNodeR2(uid)
 
+		b = append(b, plot.GlyphBox{})
 		b[i].X = plt.X.Norm(ur2.Coord2.X)
 		b[i].Y = plt.Y.Norm(ur2.Coord2.Y)
 		r := radius

graph/nodes_edges.go

@@ -56,28 +56,29 @@ type NodeSlicer interface {
 
 // NodesOf returns it.Len() nodes from it. If it is a NodeSlicer, the NodeSlice method
 // is used to obtain the nodes. It is safe to pass a nil Nodes to NodesOf.
-//
-// If the Nodes has an indeterminate length, NodesOf will panic.
 func NodesOf(it Nodes) []Node {
 	if it == nil {
 		return nil
 	}
-	len := it.Len()
+	n := it.Len()
 	switch {
-	case len == 0:
+	case n == 0:
 		return nil
-	case len < 0:
-		panic("graph: called NodesOf on indeterminate iterator")
+	case n < 0:
+		n = 0
 	}
 	switch it := it.(type) {
 	case NodeSlicer:
 		return it.NodeSlice()
 	}
-	n := make([]Node, 0, len)
+	nodes := make([]Node, 0, n)
 	for it.Next() {
-		n = append(n, it.Node())
+		nodes = append(nodes, it.Node())
 	}
-	return n
+	if len(nodes) == 0 {
+		return nil
+	}
+	return nodes
 }
 
 // Edges is an Edge iterator.
@@ -101,28 +102,29 @@ type EdgeSlicer interface {
 
 // EdgesOf returns it.Len() nodes from it. If it is an EdgeSlicer, the EdgeSlice method is used
 // to obtain the edges. It is safe to pass a nil Edges to EdgesOf.
-//
-// If the Edges has an indeterminate length, EdgesOf will panic.
 func EdgesOf(it Edges) []Edge {
 	if it == nil {
 		return nil
 	}
-	len := it.Len()
+	n := it.Len()
 	switch {
-	case len == 0:
+	case n == 0:
 		return nil
-	case len < 0:
-		panic("graph: called EdgesOf on indeterminate iterator")
+	case n < 0:
+		n = 0
 	}
 	switch it := it.(type) {
 	case EdgeSlicer:
 		return it.EdgeSlice()
 	}
-	e := make([]Edge, 0, len)
+	edges := make([]Edge, 0, n)
 	for it.Next() {
-		e = append(e, it.Edge())
+		edges = append(edges, it.Edge())
 	}
-	return e
+	if len(edges) == 0 {
+		return nil
+	}
+	return edges
 }
 
 // WeightedEdges is a WeightedEdge iterator.
@@ -147,28 +149,29 @@ type WeightedEdgeSlicer interface {
 // WeightedEdgesOf returns it.Len() weighted edge from it. If it is a WeightedEdgeSlicer, the
 // WeightedEdgeSlice method is used to obtain the edges. It is safe to pass a nil WeightedEdges
 // to WeightedEdgesOf.
-//
-// If the WeightedEdges has an indeterminate length, WeightedEdgesOf will panic.
 func WeightedEdgesOf(it WeightedEdges) []WeightedEdge {
 	if it == nil {
 		return nil
 	}
-	len := it.Len()
+	n := it.Len()
 	switch {
-	case len == 0:
+	case n == 0:
 		return nil
-	case len < 0:
-		panic("graph: called WeightedEdgesOf on indeterminate iterator")
+	case n < 0:
+		n = 0
 	}
 	switch it := it.(type) {
 	case WeightedEdgeSlicer:
 		return it.WeightedEdgeSlice()
 	}
-	e := make([]WeightedEdge, 0, len)
+	edges := make([]WeightedEdge, 0, n)
 	for it.Next() {
-		e = append(e, it.WeightedEdge())
+		edges = append(edges, it.WeightedEdge())
 	}
-	return e
+	if len(edges) == 0 {
+		return nil
+	}
+	return edges
 }
 
 // Lines is a Line iterator.
@@ -192,28 +195,29 @@ type LineSlicer interface {
 
 // LinesOf returns it.Len() nodes from it. If it is a LineSlicer, the LineSlice method is used
 // to obtain the lines. It is safe to pass a nil Lines to LinesOf.
-//
-// If the Lines has an indeterminate length, LinesOf will panic.
 func LinesOf(it Lines) []Line {
 	if it == nil {
 		return nil
 	}
-	len := it.Len()
+	n := it.Len()
 	switch {
-	case len == 0:
+	case n == 0:
 		return nil
-	case len < 0:
-		panic("graph: called LinesOf on indeterminate iterator")
+	case n < 0:
+		n = 0
 	}
 	switch it := it.(type) {
 	case LineSlicer:
 		return it.LineSlice()
 	}
-	l := make([]Line, 0, len)
+	lines := make([]Line, 0, n)
 	for it.Next() {
-		l = append(l, it.Line())
+		lines = append(lines, it.Line())
 	}
-	return l
+	if len(lines) == 0 {
+		return nil
+	}
+	return lines
 }
 
 // WeightedLines is a WeightedLine iterator.
@@ -238,28 +242,29 @@ type WeightedLineSlicer interface {
 // WeightedLinesOf returns it.Len() weighted line from it. If it is a WeightedLineSlicer, the
 // WeightedLineSlice method is used to obtain the lines. It is safe to pass a nil WeightedLines
 // to WeightedLinesOf.
-//
-// If the WeightedLines has an indeterminate length, WeightedLinesOf will panic.
 func WeightedLinesOf(it WeightedLines) []WeightedLine {
 	if it == nil {
 		return nil
 	}
-	len := it.Len()
+	n := it.Len()
 	switch {
-	case len == 0:
+	case n == 0:
 		return nil
-	case len < 0:
-		panic("graph: called WeightedLinesOf on indeterminate iterator")
+	case n < 0:
+		n = 0
 	}
 	switch it := it.(type) {
 	case WeightedLineSlicer:
 		return it.WeightedLineSlice()
 	}
-	l := make([]WeightedLine, 0, len)
+	lines := make([]WeightedLine, 0, n)
 	for it.Next() {
-		l = append(l, it.WeightedLine())
+		lines = append(lines, it.WeightedLine())
 	}
-	return l
+	if len(lines) == 0 {
+		return nil
+	}
+	return lines
 }
 
 // Empty is an empty set of nodes, edges or lines. It should be used when

graph/path/a_star_test.go

@@ -194,8 +194,9 @@ func TestExhaustiveAStar(t *testing.T) {
 	}
 
 	ps := DijkstraAllPaths(g)
-	for _, start := range graph.NodesOf(g.Nodes()) {
-		for _, goal := range graph.NodesOf(g.Nodes()) {
+	ends := graph.NodesOf(g.Nodes())
+	for _, start := range ends {
+		for _, goal := range ends {
 			pt, _ := AStar(start, goal, g, heuristic)
 			gotPath, gotWeight := pt.To(goal.ID())
 			wantPath, wantWeight, _ := ps.Between(start.ID(), goal.ID())

graph/path/dynamic/dstarlite_test.go

@@ -401,9 +401,10 @@ var dynamicDStarLiteTests = []struct {
 			l.Known[l.NodeAt(wallRow, wallCol).ID()] = false
 
 			// Check we have a correctly modified representation.
-			for _, u := range graph.NodesOf(l.Nodes()) {
+			nodes := graph.NodesOf(l.Nodes())
+			for _, u := range nodes {
 				uid := u.ID()
-				for _, v := range graph.NodesOf(l.Nodes()) {
+				for _, v := range nodes {
 					vid := v.ID()
 					if l.HasEdgeBetween(uid, vid) != l.Grid.HasEdgeBetween(uid, vid) {
 						ur, uc := l.RowCol(uid)

graph/path/internal/testgraphs/limited_test.go

@@ -1167,11 +1167,12 @@ func TestLimitedVisionGrid(t *testing.T) {
 		l.Grid.AllowDiagonal = test.diag
 
 		x, y := l.XY(test.path[0].ID())
-		for _, u := range graph.NodesOf(l.Nodes()) {
+		nodes := graph.NodesOf(l.Nodes())
+		for _, u := range nodes {
 			uid := u.ID()
 			ux, uy := l.XY(uid)
 			uNear := math.Hypot(x-ux, y-uy) <= test.radius
-			for _, v := range graph.NodesOf(l.Nodes()) {
+			for _, v := range nodes {
 				vid := v.ID()
 				vx, vy := l.XY(vid)
 				vNear := math.Hypot(x-vx, y-vy) <= test.radius

graph/path/johnson_apsp.go

@@ -152,6 +152,9 @@ func (it *johnsonNodeIterator) Len() int {
 	var len int
 	if it.nodes != nil {
 		len = it.nodes.Len()
+		if len < 0 {
+			return len
+		}
 	}
 	if !it.qUsed {
 		len++

graph/product/product_test.go

@@ -86,11 +86,11 @@ func TestCartesian(t *testing.T) {
 		naiveCartesian(want, test.a, test.b)
 		wantBytes, _ := dot.Marshal(want, "", "", "  ")
 
-		gotEdgesLen := got.Edges().Len()
-		nA := test.a.Nodes().Len()
-		mA := test.a.Edges().Len()
-		nB := test.b.Nodes().Len()
-		mB := test.b.Edges().Len()
+		gotEdgesLen := len(graph.EdgesOf(got.Edges()))
+		nA := len(graph.NodesOf(test.a.Nodes()))
+		mA := len(graph.EdgesOf(test.a.Edges()))
+		nB := len(graph.NodesOf(test.b.Nodes()))
+		mB := len(graph.EdgesOf(test.b.Edges()))
 		wantEdgesLen := mB*nA + mA*nB
 		if gotEdgesLen != wantEdgesLen {
 			t.Errorf("unexpected number of edges for Cartesian product of %s: got:%d want:%d",
@@ -133,9 +133,9 @@ func TestTensor(t *testing.T) {
 		naiveTensor(want, test.a, test.b)
 		wantBytes, _ := dot.Marshal(want, "", "", "  ")
 
-		gotEdgesLen := got.Edges().Len()
-		mA := test.a.Edges().Len()
-		mB := test.b.Edges().Len()
+		gotEdgesLen := len(graph.EdgesOf(got.Edges()))
+		mA := len(graph.EdgesOf(test.a.Edges()))
+		mB := len(graph.EdgesOf(test.b.Edges()))
 		wantEdgesLen := 2 * mA * mB
 		if gotEdgesLen != wantEdgesLen {
 			t.Errorf("unexpected number of edges for Tensor product of %s: got:%d want:%d",
@@ -178,11 +178,11 @@ func TestLexicographical(t *testing.T) {
 		naiveLexicographical(want, test.a, test.b)
 		wantBytes, _ := dot.Marshal(want, "", "", "  ")
 
-		gotEdgesLen := got.Edges().Len()
-		nA := test.a.Nodes().Len()
-		mA := test.a.Edges().Len()
-		nB := test.b.Nodes().Len()
-		mB := test.b.Edges().Len()
+		gotEdgesLen := len(graph.EdgesOf(got.Edges()))
+		nA := len(graph.NodesOf(test.a.Nodes()))
+		mA := len(graph.EdgesOf(test.a.Edges()))
+		nB := len(graph.NodesOf(test.b.Nodes()))
+		mB := len(graph.EdgesOf(test.b.Edges()))
 		wantEdgesLen := mB*nA + mA*nB*nB
 		if gotEdgesLen != wantEdgesLen {
 			t.Errorf("unexpected number of edges for Lexicographical product of %s: got:%d want:%d",
@@ -225,11 +225,11 @@ func TestStrong(t *testing.T) {
 		naiveStrong(want, test.a, test.b)
 		wantBytes, _ := dot.Marshal(want, "", "", "  ")
 
-		gotEdgesLen := got.Edges().Len()
-		nA := test.a.Nodes().Len()
-		mA := test.a.Edges().Len()
-		nB := test.b.Nodes().Len()
-		mB := test.b.Edges().Len()
+		gotEdgesLen := len(graph.EdgesOf(got.Edges()))
+		nA := len(graph.NodesOf(test.a.Nodes()))
+		mA := len(graph.EdgesOf(test.a.Edges()))
+		nB := len(graph.NodesOf(test.b.Nodes()))
+		mB := len(graph.EdgesOf(test.b.Edges()))
 		wantEdgesLen := nA*mB + nB*mA + 2*mA*mB
 		if gotEdgesLen != wantEdgesLen {
 			t.Errorf("unexpected number of edges for Strong product of %s: got:%d want:%d",

graph/spectral/laplacian.go

@@ -84,7 +84,12 @@ func NewSymNormLaplacian(g graph.Undirected) Laplacian {
 				panic("network: self edge in graph")
 			}
 			if uid < vid {
-				l.SetSym(indexOf[vid], j, -1/(squdeg*math.Sqrt(float64(g.From(vid).Len()))))
+				to := g.From(vid)
+				k := to.Len()
+				if k < 0 {
+					k = len(graph.NodesOf(to))
+				}
+				l.SetSym(indexOf[vid], j, -1/(squdeg*math.Sqrt(float64(k))))
 			}
 		}
 	}

graph/testgraph/testgraph.go

@@ -1370,7 +1370,7 @@ func AddNodes(t *testing.T, g NodeAdder, n int) {
 	var addedNodes []graph.Node
 	for i := 0; i < n; i++ {
 		node := g.NewNode()
-		prev := g.Nodes().Len()
+		prev := len(graph.NodesOf(g.Nodes()))
 		if g.Node(node.ID()) != nil {
 			curr := g.Nodes().Len()
 			if curr != prev {
@@ -1380,7 +1380,7 @@ func AddNodes(t *testing.T, g NodeAdder, n int) {
 		}
 		g.AddNode(node)
 		addedNodes = append(addedNodes, node)
-		curr := g.Nodes().Len()
+		curr := len(graph.NodesOf(g.Nodes()))
 		if curr != prev+1 {
 			t.Fatalf("AddNode failed to mutate graph: curr graph order != prev graph order+1, %d != %d", curr, prev+1)
 		}
@@ -1425,9 +1425,9 @@ func AddArbitraryNodes(t *testing.T, g NodeAdder, add graph.Nodes) {
 
 	for add.Next() {
 		node := add.Node()
-		prev := g.Nodes().Len()
+		prev := len(graph.NodesOf(g.Nodes()))
 		g.AddNode(node)
-		curr := g.Nodes().Len()
+		curr := len(graph.NodesOf(g.Nodes()))
 		if curr != prev+1 {
 			t.Fatalf("AddNode failed to mutate graph: curr graph order != prev graph order+1, %d != %d", curr, prev+1)
 		}
@@ -1517,9 +1517,9 @@ func RemoveNodes(t *testing.T, g NodeRemover) {
 		}
 		first = false
 
-		prev := g.Nodes().Len()
+		prev := len(graph.NodesOf(g.Nodes()))
 		g.RemoveNode(u.ID())
-		curr := g.Nodes().Len()
+		curr := len(graph.NodesOf(g.Nodes()))
 		if curr != prev-1 {
 			t.Fatalf("RemoveNode failed to mutate graph: curr graph order != prev graph order-1, %d != %d", curr, prev-1)
 		}