Skip to content

Commit dcd4b90

Browse files
authored
Use one rule for NaN and infinite edge weights (#285)
Nothing checked for NaN or infinite weights, and three functions gave wrong answers with them. FloydWarshall kept a NaN weight as the distance even when another path existed. BellmanFord missed a negative cycle once the distances reached -Inf, since -Inf can't be relaxed. The closest-first iterator put a NaN distance in its queue, which broke the order of the vertices around it. The path and traverse functions now follow one rule: - An edge with a NaN weight is skipped, as if it weren't in the graph. - A path through a +Inf weight is as long as no path. The closest-first iterator visits such a vertex after every vertex at a finite distance. - In BellmanFord and FloydWarshall, a distance of -Inf, from a -Inf weight or from weights that add up below the float64 range, returns ErrNegativeWeightCycle. DijkstraMultiSource reported a vertex behind a +Inf edge as reached, at distance +Inf, while Dijkstra reports it as unreachable. It now matches Dijkstra, as its doc says. The VertexPriorityQueue in util orders a NaN priority after every number. Fixes #262
1 parent bf00a74 commit dcd4b90

10 files changed

Lines changed: 365 additions & 16 deletions

‎path/bellman_ford.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ var (
3838
// distances from the source vertex to all other vertices. If there is a negative weight
3939
// cycle, the algorithm typically returns an indication of this fact.
4040
//
41+
// An edge with a NaN weight is skipped, as if it weren't in the graph. A path
42+
// through a +Inf weight is as long as no path. A distance of -Inf, from a -Inf
43+
// weight or from weights that add up below the float64 range, returns
44+
// ErrNegativeWeightCycle.
45+
//
4146
// The time complexity of the Bellman-Ford algorithm is O(V*E), where V is the number of vertices
4247
// and E is the number of edges.
4348
//
@@ -75,6 +80,12 @@ func BellmanFord[T comparable](g gograph.Graph[T], start T) (map[T]float64, erro
7580
}
7681
}
7782

83+
// -Inf can't be relaxed any further, so the extra pass misses a negative
84+
// cycle once the distances reach it.
85+
if hasNegativeInfinity(dist) {
86+
return nil, ErrNegativeWeightCycle
87+
}
88+
7889
for _, edge := range edges {
7990
if dist[edge.Source().Label()] != maxValue &&
8091
dist[edge.Source().Label()]+edge.Weight() < dist[edge.Destination().Label()] {
@@ -84,3 +95,12 @@ func BellmanFord[T comparable](g gograph.Graph[T], start T) (map[T]float64, erro
8495

8596
return dist, nil
8697
}
98+
99+
func hasNegativeInfinity[T comparable](dist map[T]float64) bool {
100+
for _, d := range dist {
101+
if math.IsInf(d, -1) {
102+
return true
103+
}
104+
}
105+
return false
106+
}

‎path/dijkstra.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ import (
1414
// The time complexity of the simple Dijkstra's algorithm implementation is O(V^2).
1515
//
1616
// It returns the shortest distances from the starting vertex to all other vertices
17-
// in the graph.
17+
// in the graph. A vertex that isn't reachable has the distance math.MaxFloat64.
18+
// An edge with a NaN weight is skipped, and a path through a +Inf weight is as
19+
// long as no path.
1820
func DijkstraSimple[T comparable](g gograph.Graph[T], start T) map[T]float64 {
1921
dist := make(map[T]float64)
2022

@@ -72,7 +74,9 @@ func newDVertex[T comparable](label T) *dVertex[T] {
7274
// The time complexity of the standard Dijkstra's algorithm with a min heap is O((E+V)logV).
7375
//
7476
// It returns the shortest distances from the starting vertex to all other vertices
75-
// in the graph.
77+
// in the graph. A vertex that isn't reachable has the distance math.MaxFloat64.
78+
// An edge with a NaN weight is skipped, and a path through a +Inf weight is as
79+
// long as no path.
7680
func Dijkstra[T comparable](g gograph.Graph[T], start T) map[T]float64 {
7781
startVertex := g.GetVertexByID(start)
7882
if startVertex == nil {

‎path/dijkstra_multi.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ func (n *NearestSources[T]) reached(v T) (int, bool) {
6868
//
6969
// Distances run from the sources along the edges. On a directed graph that
7070
// is the distance from a source to the vertex. Undirected edges work both
71-
// ways. Edge weights are read the way Dijkstra reads them. The graph does
72-
// not have to be created with Weighted, and an edge without a weight counts
73-
// as 0.
71+
// ways. Edge weights are read the way Dijkstra reads them: an edge with a
72+
// NaN weight is skipped, and a path through a +Inf weight is as long as no
73+
// path. The graph does not have to be created with Weighted, and an edge
74+
// without a weight counts as 0.
7475
//
7576
// A negative edge weight returns ErrNegativeWeight. The check covers every
7677
// edge, including edges no source reaches. A source that is not in the graph
@@ -229,7 +230,7 @@ func continueNearest(adj sourceAdj, dist []float64, rank, origin, pred []int, st
229230
for e := adj.off[v]; e < adj.off[v+1]; e++ {
230231
to := adj.to[e]
231232
next := dist[v] + adj.weight[e]
232-
if !closerSource(next, rank[v], dist[to], rank[to]) {
233+
if math.IsInf(next, 1) || !closerSource(next, rank[v], dist[to], rank[to]) {
233234
continue
234235
}
235236
dist[to] = next

‎path/floyd_warshall.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ import (
3232
// between all pairs of vertices. If there is a negative weight cycle, it might not produce
3333
// the correct shortest paths, but it can still detect the presence of such cycles.
3434
//
35+
// An edge with a NaN weight is skipped, as if it weren't in the graph. A path
36+
// through a +Inf weight is as long as no path. A distance of -Inf, from a -Inf
37+
// weight or from weights that add up below the float64 range, returns
38+
// ErrNegativeWeightCycle.
39+
//
3540
// The time complexity of the Floyd-Warshall algorithm is O(V^3), where V is the
3641
// number of vertices in the graph. Despite its cubic time complexity, it is often
3742
// preferred over other algorithms like Bellman-Ford for dense graphs or when the
@@ -62,7 +67,7 @@ func FloydWarshall[T comparable](g gograph.Graph[T]) (map[T]map[T]float64, error
6267
destMap[dest.Label()] = 0
6368
}
6469

65-
if edge := g.GetEdge(source, dest); edge != nil {
70+
if edge := g.GetEdge(source, dest); edge != nil && !math.IsNaN(edge.Weight()) {
6671
if source.Label() != dest.Label() || edge.Weight() < 0 {
6772
destMap[dest.Label()] = edge.Weight()
6873
}
@@ -88,6 +93,9 @@ func FloydWarshall[T comparable](g gograph.Graph[T]) (map[T]map[T]float64, error
8893
if dist[v.Label()][v.Label()] < 0 { // Check for negative cycles using diagonal
8994
return nil, ErrNegativeWeightCycle
9095
}
96+
if hasNegativeInfinity(dist[v.Label()]) {
97+
return nil, ErrNegativeWeightCycle
98+
}
9199

92100
for _, edge := range edges {
93101
if dist[v.Label()][edge.Source().Label()] != maxValue &&

‎path/k_center.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,11 @@ var (
4141
// centers and radius 0.
4242
//
4343
// Distances follow the edges. The graph does not have to be weighted, and
44-
// an edge without a weight counts as 0. A negative weight returns
45-
// ErrNegativeWeight. k below 1, or below the number of fixed centers,
46-
// returns ErrInvalidK. A fixed center that is not in the graph returns
47-
// gograph.ErrVertexDoesNotExist.
44+
// an edge without a weight counts as 0. An edge with a NaN weight is
45+
// skipped, and a path through a +Inf weight is as long as no path. A
46+
// negative weight returns ErrNegativeWeight. k below 1, or below the number
47+
// of fixed centers, returns ErrInvalidK. A fixed center that is not in the
48+
// graph returns gograph.ErrVertexDoesNotExist.
4849
//
4950
// Time is O(k (V + E) log V) and extra space is O(V + E). The distances
5051
// from one round are kept. Opening a center searches only the vertices it

‎path/non_finite_weights_test.go‎

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
package path
2+
3+
import (
4+
"errors"
5+
"math"
6+
"testing"
7+
8+
"github.com/hmdsefi/gograph"
9+
)
10+
11+
type weightedEdge struct {
12+
from, to string
13+
weight float64
14+
}
15+
16+
// directedWeighted returns a directed, weighted graph with the given edges.
17+
// The vertices are added in the order they first appear.
18+
func directedWeighted(edges ...weightedEdge) gograph.Graph[string] {
19+
g := gograph.New[string](gograph.Directed(), gograph.Weighted())
20+
for _, e := range edges {
21+
_, _ = g.AddEdge(gograph.NewVertex(e.from), gograph.NewVertex(e.to), gograph.WithEdgeWeight(e.weight))
22+
}
23+
return g
24+
}
25+
26+
// distanceFunc returns the distance from the start vertex to v, and false
27+
// when the function reports v as unreachable.
28+
type distanceFunc func(v string) (float64, bool)
29+
30+
// shortestPaths runs every single-source shortest-path function on the
31+
// same graph. Each one reports an unreachable vertex in its own way.
32+
var shortestPaths = []struct {
33+
name string
34+
run func(g gograph.Graph[string], start string) (distanceFunc, error)
35+
}{
36+
{
37+
name: "FloydWarshall",
38+
run: func(g gograph.Graph[string], start string) (distanceFunc, error) {
39+
dist, err := FloydWarshall(g)
40+
if err != nil {
41+
return nil, err
42+
}
43+
return func(v string) (float64, bool) {
44+
return dist[start][v], !math.IsInf(dist[start][v], 1)
45+
}, nil
46+
},
47+
},
48+
{
49+
name: "BellmanFord",
50+
run: func(g gograph.Graph[string], start string) (distanceFunc, error) {
51+
dist, err := BellmanFord(g, start)
52+
if err != nil {
53+
return nil, err
54+
}
55+
return func(v string) (float64, bool) {
56+
return dist[v], !math.IsInf(dist[v], 1)
57+
}, nil
58+
},
59+
},
60+
{
61+
name: "Dijkstra",
62+
run: func(g gograph.Graph[string], start string) (distanceFunc, error) {
63+
dist := Dijkstra(g, start)
64+
return func(v string) (float64, bool) {
65+
return dist[v], dist[v] != math.MaxFloat64
66+
}, nil
67+
},
68+
},
69+
{
70+
name: "DijkstraSimple",
71+
run: func(g gograph.Graph[string], start string) (distanceFunc, error) {
72+
dist := DijkstraSimple(g, start)
73+
return func(v string) (float64, bool) {
74+
return dist[v], dist[v] != math.MaxFloat64
75+
}, nil
76+
},
77+
},
78+
{
79+
name: "DijkstraMultiSource",
80+
run: func(g gograph.Graph[string], start string) (distanceFunc, error) {
81+
nearest, err := DijkstraMultiSource(g, start)
82+
if err != nil {
83+
return nil, err
84+
}
85+
return nearest.Distance, nil
86+
},
87+
},
88+
}
89+
90+
func TestShortestPaths_NaNWeight(t *testing.T) {
91+
// the NaN edge to b is skipped, so b is reached through c, and d,
92+
// which only a NaN edge leads to, is unreachable
93+
g := directedWeighted(
94+
weightedEdge{"a", "b", math.NaN()},
95+
weightedEdge{"a", "c", 1},
96+
weightedEdge{"c", "b", 1},
97+
weightedEdge{"a", "d", math.NaN()},
98+
)
99+
100+
for _, tt := range shortestPaths {
101+
t.Run(tt.name, func(t *testing.T) {
102+
distance, err := tt.run(g, "a")
103+
if err != nil {
104+
t.Fatalf("unexpected error: %v", err)
105+
}
106+
if d, ok := distance("b"); !ok || d != 2 {
107+
t.Errorf("b: expected distance 2, got %v (reachable %v)", d, ok)
108+
}
109+
if d, ok := distance("d"); ok {
110+
t.Errorf("d: expected it to be unreachable, got distance %v", d)
111+
}
112+
})
113+
}
114+
}
115+
116+
func TestShortestPaths_PositiveInfinityWeight(t *testing.T) {
117+
// a path through a +Inf edge is as long as no path
118+
g := directedWeighted(
119+
weightedEdge{"a", "b", math.Inf(1)},
120+
weightedEdge{"a", "c", 1},
121+
weightedEdge{"b", "d", 1},
122+
)
123+
124+
for _, tt := range shortestPaths {
125+
t.Run(tt.name, func(t *testing.T) {
126+
distance, err := tt.run(g, "a")
127+
if err != nil {
128+
t.Fatalf("unexpected error: %v", err)
129+
}
130+
if d, ok := distance("c"); !ok || d != 1 {
131+
t.Errorf("c: expected distance 1, got %v (reachable %v)", d, ok)
132+
}
133+
for _, v := range []string{"b", "d"} {
134+
if d, ok := distance(v); ok {
135+
t.Errorf("%s: expected it to be unreachable, got distance %v", v, d)
136+
}
137+
}
138+
})
139+
}
140+
}
141+
142+
func TestShortestPaths_NegativeInfinityDistance(t *testing.T) {
143+
tests := []struct {
144+
name string
145+
edges []weightedEdge
146+
}{
147+
{
148+
name: "negative infinite weight in a cycle",
149+
edges: []weightedEdge{{"a", "b", math.Inf(-1)}, {"b", "a", 1}},
150+
},
151+
{
152+
name: "negative infinite weight",
153+
edges: []weightedEdge{{"a", "b", math.Inf(-1)}, {"b", "c", 1}},
154+
},
155+
{
156+
name: "negative cycle that sums below the float64 range",
157+
edges: []weightedEdge{{"a", "b", -1e308}, {"b", "a", -1e308}, {"c", "c", 0}},
158+
},
159+
{
160+
name: "path that sums below the float64 range",
161+
edges: []weightedEdge{{"a", "b", -1e308}, {"b", "c", -1e308}},
162+
},
163+
}
164+
165+
for _, tt := range tests {
166+
t.Run(tt.name, func(t *testing.T) {
167+
g := directedWeighted(tt.edges...)
168+
if dist, err := BellmanFord(g, "a"); !errors.Is(err, ErrNegativeWeightCycle) {
169+
t.Errorf("BellmanFord: expected ErrNegativeWeightCycle, got %v and %v", dist, err)
170+
}
171+
if dist, err := FloydWarshall(g); !errors.Is(err, ErrNegativeWeightCycle) {
172+
t.Errorf("FloydWarshall: expected ErrNegativeWeightCycle, got %v and %v", dist, err)
173+
}
174+
if _, err := DijkstraMultiSource(g, "a"); !errors.Is(err, ErrNegativeWeight) {
175+
t.Errorf("DijkstraMultiSource: expected ErrNegativeWeight, got %v", err)
176+
}
177+
})
178+
}
179+
}
180+
181+
func TestKCenter_NonFiniteWeights(t *testing.T) {
182+
tests := []struct {
183+
name string
184+
edges []weightedEdge
185+
radius float64
186+
}{
187+
{
188+
// b is reached through c, past the NaN edge
189+
name: "NaN weight",
190+
edges: []weightedEdge{{"a", "b", math.NaN()}, {"a", "c", 1}, {"c", "b", 1}},
191+
radius: 2,
192+
},
193+
{
194+
// b is only behind the +Inf edge, so a can't cover it
195+
name: "positive infinite weight",
196+
edges: []weightedEdge{{"a", "b", math.Inf(1)}, {"a", "c", 1}},
197+
radius: math.Inf(1),
198+
},
199+
}
200+
201+
for _, tt := range tests {
202+
t.Run(tt.name, func(t *testing.T) {
203+
centers, radius, err := KCenter(directedWeighted(tt.edges...), 1, "a")
204+
if err != nil {
205+
t.Fatalf("unexpected error: %v", err)
206+
}
207+
if len(centers) != 1 || centers[0].Label() != "a" {
208+
t.Fatalf("expected the fixed center a, got %v", centers)
209+
}
210+
if radius != tt.radius {
211+
t.Fatalf("expected radius %v, got %v", tt.radius, radius)
212+
}
213+
})
214+
}
215+
}

‎traverse/closest_first_iterator.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package traverse
22

33
import (
44
"container/heap"
5+
"math"
56

67
"github.com/hmdsefi/gograph"
78
)
@@ -103,6 +104,10 @@ type closestFirstIterator[T comparable] struct {
103104
// NewClosestFirstIterator creates a new instance of closestFirstIterator
104105
// and returns it as the Iterator interface.
105106
//
107+
// Edge weights are expected to be non-negative. An edge with a NaN weight is
108+
// skipped. A vertex reached only through a +Inf weight is visited after every
109+
// vertex at a finite distance.
110+
//
106111
// if the start node doesn't exist, returns error.
107112
func NewClosestFirstIterator[T comparable](graph gograph.Graph[T], start T) (Iterator[T], error) {
108113
v := graph.GetVertexByID(start)
@@ -154,8 +159,11 @@ func (c *closestFirstIterator[T]) Next() *gograph.Vertex[T] {
154159
continue
155160
}
156161
edge := c.graph.GetEdge(currNode, neighbor)
157-
if edge != nil {
158-
dist := c.currDist + edge.Weight()
162+
if edge == nil {
163+
continue
164+
}
165+
// the queue can't order a NaN distance
166+
if dist := c.currDist + edge.Weight(); !math.IsNaN(dist) {
159167
c.pq.pushOrDecrease(c.graph.GetVertexByID(neighbor.Label()), dist)
160168
}
161169
}

0 commit comments

Comments
 (0)