Dijkstra's algorithm: is my implementation flawed?

Question

In order to train myself both in Python and graph theory, I tried to implement the Dijkstra algo using Python 3, and submitted it against several online judges, to see if it was correct.

It works well in many cases, but not always.

For example, I am stuck with this one: the test case works fine and I also have tried custom test cases of my own, but when I submit the following solution, the judge keeps telling me "wrong answer", and the expected result is very different from my output, indeed.

Notice that the judge tests it against quite a complex graph (10000 nodes with 100000 edges), while all the cases I tried before never exceeded 20 nodes and around 20-40 edges.

Here is my code.

Given al an adjacency list in the following form:

al[n] = [(a1, w1), (a2, w2), ...]

where

n is the node id;
a1, a2, etc. are its adjacent nodes and w1, w2, etc. the respective weights for the given edge;

and supposing that maximum distance never exceeds 1 billion, I implemented Dijkstra's algorithm this way:

import queue

distance = [1000000000] * (N+1) # this is the array where I store the shortest path between 1 and each other node
distance[1] = 0 # starting from node 1 with distance 0

pq = queue.PriorityQueue()
pq.put((0, 1)) # same as above

visited = [False] * (N+1) 

while not pq.empty():
    n = pq.get()[1]
    if visited[n]:
        continue
    visited[n] = True
    for edge in al[n]:
        if distance[edge[0]] > distance[n] + edge[1]:
            distance[edge[0]] = distance[n] + edge[1]
            pq.put((distance[edge[0]], edge[0]))

Could you please help me understand wether my implementation is flawed, or if I simply ran into some bugged online judge?

Thank you very much.

UPDATE

As requested, I'm providing the snippet I use to populate the adjacency list al for the linked problem.

N,M = input().split()
N,M = int(N), int(M)

al = [[] for n in range(N+1)]

for m in range(M):
    try:
        a,b,w = input().split()
        a,b,w = int(a), int(b), int(w)
        al[a].append((b, w))
        al[b].append((a, w))
    except:
       pass

(Please don't mind the ugly "except: pass", I was using it just for debugging purposes... :P)

I'm not familiar with this subject, so I can't say if this is intentional or not(sorry if it is) but are you aware that, all items in `distance` is referencing same object? Which means, if you change one, you change all. — Lafexlos, Aug 11 '16 at 14:13
As far as I remember in this algorithm you should not do anything when child node is already visited. So after `for edge in al[n]:` there should be `if visited[edge]: continue`. But I might be wrong. Try to test your algorithm on full graph as edge test case. — mbednarski, Aug 11 '16 at 14:17
@Lafexlos While list multiplication can yield this problem, in this case it's okay. The items in the list are not all the same -- not least since those are immutable numbers. — tobias_k, Aug 11 '16 at 14:19
@tobias_k Ah. No idea why but I thought they are all lists and it's list of lists. Meh, sorry. — Lafexlos, Aug 11 '16 at 14:22
You are using the _weight_ of the edges in your queue `pq.put((edge[1], edge[0]))`. Shouldn't you be using the total distance to the node? `pq.put((distance[edge[0]], edge[0]))` — tobias_k, Aug 11 '16 at 14:24
@Xevaquor: I remember that you should also update already visited nodes in case a shorter distance comes up... — javatutorial, Aug 11 '16 at 14:25
@tobias_k: oh, s**t. THAT might be the problem, indeed. I'll try. Thanks. — javatutorial, Aug 11 '16 at 14:26
@tobias_k: tried, but no luck. I'm updating the question, anyway. Thanks for pointing out this issue. — javatutorial, Aug 11 '16 at 14:30
Can you add some (larger) sample input and output data? How does the "sample input" from the linked page translate to adjacency lists? — tobias_k, Aug 11 '16 at 14:32
@tobias_k: question updated. And I hope my problem is not a simple parsing error... xD — javatutorial, Aug 11 '16 at 14:37
Doesn't the line "It works well in many cases, but not always" answer your question "is my implementation flawed?" — John Coleman, Aug 11 '16 at 14:39
@JohnColeman: the fact is that my implementation might be buggy, but the same consideration might apply to the online judge :) since I cannot see my flaw, I'm asking for help in case you see it. ^^ — javatutorial, Aug 11 '16 at 14:42
According to your parsing code, you are treating the input data as an _undirected_ graph, i.e. each edge from A to B also is an edge from B to A. Is this premise valid? Shouldn't it be a _directed_ graph? — tobias_k, Aug 11 '16 at 14:44
@tobias_k: again, you are right. The statement indeed talks about "an edge from a vertex ai to a vertex bi"... It was me who was assuming it was an undirected edge. Going to try and correct that snippet. — javatutorial, Aug 11 '16 at 14:50
@tobias_k: aaaaaand we have a winner. :) Thanks. I should really read problem statements more carefully. If you want to setup a quick answer I can upvote and accept it. — javatutorial, Aug 11 '16 at 14:52

tobias_k · Accepted Answer · 2016-08-11T14:53:29.883

Primary problem in interpreting the question:

According to your parsing code, you are treating the input data as an undirected graph, i.e. each edge from A to B also is an edge from B to A. Is seems like this premise is not valid and it should instead be a directed graph, i.e. you have to remove this line:

    al[b].append((a, w))  # no back reference!

Previous problem, now already fixed in the code:

Currently, you are using the never-changing weight of the edges in your queue:

        pq.put((edge[1], edge[0]))

This way, the nodes always end up at the same position in the queue, no matter at what stage of the algorithm and how far the path to reach that node actually is. Instead, you should use the new distance to the target node edge[0], i.e. distance[edge[0]] as the priority in the queue:

        pq.put((distance[edge[0]], edge[0]))

Dijkstra's algorithm: is my implementation flawed?

1 Answers1