KSP fixes.

jrecursive · Apr 26, 2013 · d3ef70f · d3ef70f
1 parent 9891701
commit d3ef70f
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 16 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -12,6 +12,7 @@ Changes to JGraphT in each version:
 	- Fix path weight bug in `FloydWarshallShortestPaths`, contributed by Michal Pasieka
 	- Add `PrimMinimumSpanningTree`, contributed by Alexey Kudinkin
 	- Add `DirectedWeightedPseudograph`, and fix 'DirectedMultigraph' contributed by Adam Gouge
+        - More KSP bugfixes (spotted by Sebastian Mueller, fixed by Guillaume Boulmier)
 
 - **version 0.8.3** (20-Jan-2012): 
 	- fix regression in `DOTExporter` inadvertently introduced by `0.8.2` changes.

diff --git a/jgrapht-core/src/main/java/org/jgrapht/alg/RankingPathElementList.java b/jgrapht-core/src/main/java/org/jgrapht/alg/RankingPathElementList.java
@@ -121,25 +121,26 @@ final class RankingPathElementList<V, E>
         this.guardVertexToNotDisconnect = guardVertexToNotDisconnect;
 
         // loop over the path elements in increasing order of weight.
-        for (int i = 0; i < elementList.size(); i++) {
+        for (int i = 0;
+             (i < elementList.size()) && (size() < maxSize); i++)
+        {
             RankingPathElement<V, E> prevPathElement = elementList.get(i);
 
             if (isNotValidPath(prevPathElement, edge)) {
+                // go to the next path element in the loop
                 continue;
             }
 
-            if (size() < this.maxSize) {
-                double weight = calculatePathWeight(prevPathElement, edge);
-                RankingPathElement<V, E> newPathElement =
-                    new RankingPathElement<V, E>(
-                        this.graph,
-                        prevPathElement,
-                        edge,
-                        weight);
-
-                // the new path is inserted at the end of the list.
-                this.pathElements.add(newPathElement);
-            }
+            double weight = calculatePathWeight(prevPathElement, edge);
+            RankingPathElement<V, E> newPathElement =
+                new RankingPathElement<V, E>(
+                    this.graph,
+                    prevPathElement,
+                    edge,
+                    weight);
+
+            // the new path is inserted at the end of the list.
+            this.pathElements.add(newPathElement);
         }
     }
 
@@ -379,14 +380,18 @@ private boolean isSimplePath(
         RankingPathElement<V, E> prevPathElement,
         E edge)
     {
+        V endVertex = Graphs.getOppositeVertex(this.graph, edge,
+            prevPathElement.getVertex());
+        assert (endVertex.equals(this.vertex));
+
         RankingPathElement<V, E> pathElementToTest = prevPathElement;
-        while (pathElementToTest.getPrevEdge() != null) {
-            if (pathElementToTest.getVertex() == this.vertex) {
+        do {
+            if (pathElementToTest.getVertex().equals(endVertex)) {
                 return false;
             } else {
                 pathElementToTest = pathElementToTest.getPrevPathElement();
             }
-        }
+        } while (pathElementToTest != null);
 
         return true;
     }

diff --git a/jgrapht-core/src/test/java/org/jgrapht/alg/KShortestPathCostTest.java b/jgrapht-core/src/test/java/org/jgrapht/alg/KShortestPathCostTest.java
@@ -39,11 +39,13 @@
  */
 package org.jgrapht.alg;
 
+import java.io.*;
 import java.util.*;
 
 import junit.framework.*;
 
 import org.jgrapht.*;
+import org.jgrapht.graph.*;
 
 
 /**
@@ -296,6 +298,62 @@ private void verifyWeightsConsistency(
                 == prevPathElementResult.getWeight());
         }
     }
+
+    /**
+     * Currently disabled since it takes more than a few seconds to run.
+     * @see <a
+     * href="http://jgrapht-users.107614.n3.nabble.com/quot-graph-must-contain-the-start-vertex-quot-when-running-KShortestPaths-td4024797.html">bug
+     * description</a>.
+     */
+    public void _testIllegalArgumentExceptionGraphNotThrown()
+        throws Exception
+    {
+        SimpleWeightedGraph<String, DefaultWeightedEdge> graph =
+            new SimpleWeightedGraph<String, DefaultWeightedEdge>(
+                DefaultWeightedEdge.class);
+
+
+        InputStream fstream = getClass().getClassLoader().getResourceAsStream(
+            "edges.txt");
+        BufferedReader in = new BufferedReader(new InputStreamReader(fstream));
+
+        String[] edgeText;
+        DefaultWeightedEdge ed;
+        String line = in.readLine();
+        while (line != null) {
+            edgeText = line.split("\t");
+
+            graph.addVertex(edgeText[0]);
+            graph.addVertex(edgeText[1]);
+            ed = graph.addEdge(edgeText[0], edgeText[1]);
+            graph.setEdgeWeight(ed, Double.parseDouble(edgeText[2]));
+
+            line = in.readLine();
+        }
+
+        // Close the input stream
+        in.close();
+
+        DefaultWeightedEdge src = graph.getEdge("M013", "M014");
+
+        KShortestPaths<String, DefaultWeightedEdge> kPaths =
+            new KShortestPaths<String, DefaultWeightedEdge>(
+                graph, graph.getEdgeSource(src), 5);
+        List<GraphPath<String, DefaultWeightedEdge>> paths = null;
+
+        try {
+            paths = kPaths.getPaths(graph.getEdgeTarget(src));
+            for (GraphPath<String, DefaultWeightedEdge> path : paths) {
+                for (DefaultWeightedEdge edge : path.getEdgeList()) {
+                    System.out.print("<" + graph.getEdgeSource(edge) + "\t"
+                        + graph.getEdgeTarget(edge) + "\t" + edge + ">\t");
+                }
+                System.out.println(": " + path.getWeight());
+            }
+        } catch (IllegalArgumentException e) {
+            fail("IllegalArgumentException thrown");
+        }
+    }
 }
 
 // End KShortestPathCostTest.java