Floyd-Warshall Cant find whats wrong

Question

I need a new set pair of eyes on this, for some reason its not generating the correct Sequence and Distance matrices the following is my implementation.

This is in C# and DistanceMatrix is a double [,] and SequenceMatrix is a string [,]

these are initiated as follows: http://puu.sh/951Tz/5ef27e3996.png

for (int k = 0; k < villageCount; k++)
        {
            for (int i = 0; i < villageCount; i++)
            {
                if (k == i)
                    continue;

                for (int j = 0; j < villageCount; j++)
                {
                    if (j == i)
                        continue;

                    if (j == k)
                        continue;

                    if (fw.DistanceMatrix[i, j] >= (fw.DistanceMatrix[i, k] + fw.DistanceMatrix[k, j]))
                    {
                        fw.DistanceMatrix[i, j] = (fw.DistanceMatrix[i, k] + fw.DistanceMatrix[k, j]);
                        fw.SequenceMatrix[i, j] = fw.SequenceMatrix[k, j];
                    }
                }
            }
        }

for some reason I'm getting the following output:

    [0, 0]  "A" string
    [0, 1]  "B" string
    [0, 2]  "A" string
    [0, 3]  "B" string
    [0, 4]  "D" string
    [1, 0]  "B" string
    [1, 1]  "D" string
    [1, 2]  "D" string
    [1, 3]  "B" string
    [1, 4]  "D" string
    [2, 0]  "B" string
    [2, 1]  "B" string
    [2, 2]  "B" string
    [2, 3]  "B" string
    [2, 4]  "D" string
    [3, 0]  "B" string
    [3, 1]  "B" string
    [3, 2]  "C" string
    [3, 3]  "C" string
    [3, 4]  "D" string
    [4, 0]  "B" string
    [4, 1]  "E" string
    [4, 2]  "D" string
    [4, 3]  "B" string
    [4, 4]  "E" string

any pointers would be much appreciated, also if you require more information i'll be F5ing this page :)

distance matrix after init

    [0, 0]  0.0                                 double
    [0, 1]  50.0                                    double
    [0, 2]  2.0                                 double
    [0, 3]  10.0                                    double
    [0, 4]  1.7976931348623157E+308 double
    [1, 0]  50.0                                    double
    [1, 1]  0.0                                 double
    [1, 2]  3.0                                 double
    [1, 3]  1.7976931348623157E+308 double
    [1, 4]  1.0                                 double
    [2, 0]  2.0                                 double
    [2, 1]  3.0                                 double
    [2, 2]  0.0                                 double
    [2, 3]  5.0                                 double
    [2, 4]  5.0                                 double
    [3, 0]  10.0                                    double
    [3, 1]  1.7976931348623157E+308 double
    [3, 2]  5.0                                 double
    [3, 3]  0.0                                 double
    [3, 4]  1.7976931348623157E+308 double
    [4, 0]  1.7976931348623157E+308 double
    [4, 1]  1.0                                 double
    [4, 2]  5.0                                 double
    [4, 3]  1.7976931348623157E+308 double
    [4, 4]  0.0                                 double

This is from a class mate of mine i gave him screen shots of my code and there are no differences that should effect the algorithm http://puu.sh/94W63/3733d3275f.png also this is the map if you are interested http://puu.sh/954sl/89bd904fda.png if you are interested this is the initiation of the matrix http://puu.sh/954Bu/fc0be21893.png — Ol1v3r, May 28 '14 at 18:20
You say in your screenshot that you init the values in `DistanceMatrix` to infinity, but do you then set the appropriate elements in it based on the connections in the graph? The part of the code you posted seems fine — Golden Dragon, May 28 '14 at 18:28
yes http://puu.sh/9554w/5bbe3027fa.png, currently i'm working out all the iterations by hand to compare the results :/ if you can save me that time :) — Ol1v3r, May 28 '14 at 18:31
So what does the distance matrix look like after you initialize it? — Golden Dragon, May 28 '14 at 18:32
In the [pseudo code](http://en.wikipedia.org/wiki/Floyd-Warshall_algorithm) for this algorithm it doesn't skip if the different iterators are equal. Have tried commenting those lines out and seeing what you get? — tinstaafl, May 28 '14 at 18:46
that is a performance modification and yes i have tried removing it still same result — Ol1v3r, May 28 '14 at 18:48
A hint for future reference. It will usually pay off to get your code working first then worry about performance. This way you always have a working base line to go back to. — tinstaafl, May 28 '14 at 18:50

Golden Dragon · Answer 1 · 2014-05-28T19:05:38.353

2

I believe I found the problem.

You are initializing SequenceMatrix incorrectly. Your current have

fw.SequenceMatrix[i, j] = map.Villages.ElementAt(i).Name;

However, as per the Wikipedia article, it should be

fw.SequenceMatrix[i, j] = map.Villages.ElementAt(**j**).Name;

Since the sequence matrix shows you the path to take, the values in it should be the destinations, not the starting points.

Looks like there is a mistake in your algorithm, too. You have

fw.SequenceMatrix[i, j] = fw.SequenceMatrix[k, j];

But it should be

fw.SequenceMatrix[i, j] = fw.SequenceMatrix[**i, k**];

Since you have determined that moving from i to k to j is shorter than moving from i to j, you want to set the first step of the new path from i to j to be the same as the first step of the path from i to k.

edited May 28 '14 at 19:05

answered May 28 '14 at 18:51

Golden Dragon

511
5
13

So you are still getting identical output to previously? – Golden Dragon May 28 '14 at 19:00
No with both of these changes the new output looks like this which is still wrong http://puu.sh/9599j/335a8c0241.png – Ol1v3r May 28 '14 at 19:22
You're going to have to debug your code and step though it to see exactly what is going wrong, then. – Golden Dragon May 28 '14 at 19:31
HeHe been doing that since 11 this morning – Ol1v3r May 28 '14 at 19:31

Ralor · Answer 2 · 2014-05-28T20:32:39.647

1

Maybe the problem is that inf+inf isn't ok (overflow)? Anyway, I think implemetations should look like this

// d[i][j] == weight of edge (i,j)
// d[i][j] == INF if there is no such edge
// d[i][i] == 0; i = 0, .., n-1

for (int k = 0; k < n; ++k)
    for (int i = 0; i < n; ++i)
        for (int j = 0; j < n; ++j)
            if (d[i][k] < INF && d[k][j] < INF) // i->k, k->j should exist
                d[i][j] = min (d[i][j], d[i][k] + d[k][j]); // INF+INF won't happen

edited May 28 '14 at 20:32

answered May 28 '14 at 20:24

Ralor

371
1
3
10

Using min prevents you from maintaining parent-pointers, such as the OP's SequenceMatrix – Gilthans May 28 '14 at 20:38

score 0 · Accepted Answer · answered May 28 '14 at 20:33

0

Fixed !!! HURRAY

There was no problem to being with !!!!! the nodes where not being Saved

A B C D in the database but rather A D B C, then when reading i was working it out on the index assuming that A is at index 0 B is at index 1 and so on....

Sorry for the waste of time, and thanks a lot !!! for your help !!

answered May 28 '14 at 20:33

Ol1v3r

758
1
10
23

glad you figured that out) saving some attributive information in database may help for such cases – Ralor May 28 '14 at 20:36
1

It is important to note that without Golden Dragon's fix below, your code still wouldn't work... Don't forget to accept his answer – Gilthans May 28 '14 at 20:40
actually its not implemented with Golden Dragon's fix and it still works, which got me a bit confused as to why, but still a big thanks for everyone's input, also that is how i got to the problem i looked at the data in the database :) – Ol1v3r May 28 '14 at 21:13

Floyd-Warshall Cant find whats wrong

3 Answers3