0

I am implementing Dijkstra's shortest path algorithm recursivingly in Scala, but I am having some trouble. I am getting the incorrect output for nodes 3 to 2, called like this, shortestPath(3, 2, x, BitSet.empty). This outputs 6, but the correct answer should be 7. I cannot seem to figure out what's wrong with my code.

var x = ListBuffer(ListBuffer(0, 2, 3, 4), 
                   ListBuffer(2, 0, 0, 0), 
                   ListBuffer(3, 0, 0, 0), 
                   ListBuffer(4, 0, 0, 0))

My code is here shown below.

def shortestPath(cur: Int, dest: Int, graph: ListBuffer[ListBuffer[Int]], visited: BitSet) :Int = {
    val newVisited = visited + cur
    if(cur == dest) 0
    else {
      var pathLength = for(i <- graph(cur).indices; if(!visited(i) && graph(cur)(i) > 0)) yield {
          graph(cur)(i) + shortestPath(i, dest, graph, newVisited)
        }
      if (pathLength.isEmpty) 0 else pathLength.min
      }
    }
Teodorico Levoff
  • 1,641
  • 2
  • 26
  • 44

2 Answers2

2

As pointed out by obourgain, the critical error of the code is at interpreting the min-distance as 0 when two nodes are not connected.

The min-distance between two nodes should be infinity if they are disconnected, this is because the cost of two disconnected nodes must be greater than the cost of any connected nodes, and one simple fix to your code is to identify infinity with Int.MaxValue.

def shortestPath(cur: Int, dest: Int, graph: ListBuffer[ListBuffer[Int]], visited: BitSet) :Int = {
    val newVisited = visited + cur
    if(cur == dest) 0
    else {
        var pathLength = for(i <- graph(cur).indices; if(!visited(i) && graph(cur)(i) > 0)) yield {
            val sLen = shortestPath(i, dest, graph, newVisited)
            if (graph(cur)(i) > Int.MaxValue - sLen) Int.MaxValue else graph(cur)(i) + sLen // change #1
        }
        if (pathLength.isEmpty) Int.MaxValue else pathLength.min // change #2
    }
}

This modification will give the expected answer Int = 7 when invoking shortestPath(3, 2, x, new BitSet()).

The code commented with "change #1" is to prevent integer overflow when the destination node is not reachable by the neighbor node (thus the min-distance is Int.MaxValue), and the code commented with "change #2" is to treat the min-distance between two nodes as "infinite" when they are disconnected.

Community
  • 1
  • 1
chiwangc
  • 3,566
  • 16
  • 26
  • 32
  • I understand now! But the algorithm presented or idea is correct? – Teodorico Levoff Nov 17 '16 at 05:18
  • @TeodoricoLevoff Just to clarify, are you trying to ask whether *your code is correct in finding the shortest path between two nodes* or whether *the trick to identify infinity with `Int.MaxValue` is correct*? – chiwangc Nov 17 '16 at 05:38
  • If the code is correct in finding the shortest path between two nodes. – Teodorico Levoff Nov 17 '16 at 05:49
  • @TeodoricoLevoff You are right, your code captures the fact `minDist(x, y) = min{ minDist(x, z) + dist(z, y) | z is a neighbor of x}`, and the cases skipped by the check against `BitSet` will not give shorter path as they visit repeated nodes. – chiwangc Nov 17 '16 at 06:41
1

The error is on the last line:

  if (pathLength.isEmpty) 0 else pathLength.min

If pathLength.isEmpty, it means the two points are not connected. However, the function returns 0, which is interpreted as a connection with weight 0.

obourgain
  • 8,856
  • 6
  • 42
  • 57