Hi Peter
Ideally I would like to stop traversal as soon as the max. cost limit is 
reached as we know there are no negative edges. That would probable be good 
performance-wise for huge graphs. Attached is a patch for my attempt at 
implementing the cost limit option. Its not thoroughly tested though I have 
added some tests as well that pass. But it would be great of it can be tested a 
little more if you decide to apply it.

Let me know what you think.

Regards
Nabeel Mukhtar



----- Original Message ----
From: Peter Neubauer <peter.neuba...@neotechnology.com>
To: Neo user discussions <user@lists.neo4j.org>
Sent: Wed, January 27, 2010 3:29:06 AM
Subject: Re: [Neo] Cost limit option in Dijkstra Algorithm

Nabeel,
you are right, this is not going to work. I am prepared to take out
the cost evaluation again since I don't want to have non-working code
in there, and have no time to figure this out right now.

Is that ok with you, or would you like to figure out a better place to
put the cost evaluation, maybe after a shortest path has been found in
order to exclude it and continue or be done if it is cheaper than
maxCost? I that case I wil be happy to apply a patch...

Cheers,

/peter neubauer

COO and Sales, Neo Technology

GTalk:      neubauer.peter
Skype       peter.neubauer
Phone       +46 704 106975
LinkedIn   http://www.linkedin.com/in/neubauer
Twitter      http://twitter.com/peterneubauer

http://www.neo4j.org                - Your high performance graph database.
http://gremlin.tinkerpop.com    - PageRank in 2 lines of code.



On Tue, Jan 26, 2010 at 10:06 AM, Nabeel Siddiqui
<nabeelmukh...@yahoo.com> wrote:
> Hi all
> Thanks Peter for adding a cost limit option to the Dijkstra algorithm in the 
> graph algo package. However for some reason I could not make it to work as I 
> expected.
> I see you have added some tests in FindPathTest#testMaxCost method (which is 
> given below):
>    public void testMaxCost()
>    {
>        graph.makeEdge( "a", "b", "cost", (double) 1 );
>        graph.makeEdge( "a", "c", "cost", (double) 1 );
>        graph.makeEdge( "c", "d", "cost", (double) 1 );
>        graph.makeEdge( "b", "d", "cost", (double) 1 );
>        FindPath findPath = new FindPath( graph.getNode( "a" ), graph
>                .getNode( "d" ), 0, Direction.OUTGOING, MyRelTypes.R1 );
>        List<List<PropertyContainer>> paths = findPath.getPaths();
>        assertTrue( paths.isEmpty() );
>        assertNull( findPath.getCost() );
>        findPath = new FindPath( graph.getNode( "a" ), graph
>                .getNode( "d" ), 1, Direction.OUTGOING, MyRelTypes.R1 );
>        assertTrue( findPath.getCost() == 2 );
>        assertTrue( findPath.getPathAsNodes().size() == 3 );
>        assertTrue( findPath.getPathAsRelationships().size() == 2 );
>    }
>
> However I expect the second test to fail as the maximum cost specified is 1 
> but the cost from a to d is greater than 1. So no paths should be returned. 
> Interestingly If I add another edge:
>        graph.makeEdge( "d", "e", "cost", (double) 1 );
>
> And search from a to e with max cost 1, it still returns the paths although 
> it should return empty list as the cost from a to e is obviously greater than 
> 1. What am I doing wrong?. As far as I know, the max cost check inside 
> DijstraIterator may not work as the algorithm uses two simultaneous iterators 
> and the total cost of both traversals may exceed the limit while the 
> individual cost is still under limit.
>
> I have attached a patch with the added test that fails.
> Also it would be great to rename the test sub-package from shortestPath to 
> shortestpath (same as the source package) because it creates problems in 
> eclipse when both src and test are in the source path.
>
> Thanks for your help.
>
> Regards
> Nabeel Mukhtar
>
>
>
>
> _______________________________________________
> Neo mailing list
> User@lists.neo4j.org
> https://lists.neo4j.org/mailman/listinfo/user
>
>
_______________________________________________
Neo mailing list
User@lists.neo4j.org
https://lists.neo4j.org/mailman/listinfo/user



      
Index: src/test/java/org/neo4j/graphalgo/shortestPath/FindPathTest.java
===================================================================
--- src/test/java/org/neo4j/graphalgo/shortestPath/FindPathTest.java    
(revision 3636)
+++ src/test/java/org/neo4j/graphalgo/shortestPath/FindPathTest.java    
(working copy)
@@ -80,16 +80,29 @@
         graph.makeEdge( "a", "c", "cost", (double) 1 );
         graph.makeEdge( "c", "d", "cost", (double) 1 );
         graph.makeEdge( "b", "d", "cost", (double) 1 );
+        graph.makeEdge( "d", "e", "cost", (double) 1 );
+        graph.makeEdge( "e", "f", "cost", (double) 1 );
         FindPath findPath = new FindPath( graph.getNode( "a" ), graph
                 .getNode( "d" ), 0, Direction.OUTGOING, MyRelTypes.R1 );
         List<List<PropertyContainer>> paths = findPath.getPaths();
                assertTrue( paths.isEmpty() );
                assertNull( findPath.getCost() );
         findPath = new FindPath( graph.getNode( "a" ), graph
-                .getNode( "d" ), 1, Direction.OUTGOING, MyRelTypes.R1 );
+                .getNode( "d" ), 2, Direction.OUTGOING, MyRelTypes.R1 );
         assertTrue( findPath.getCost() == 2 );
         assertTrue( findPath.getPathAsNodes().size() == 3 );
         assertTrue( findPath.getPathAsRelationships().size() == 2 );
+
+        findPath = new FindPath( graph.getNode( "a" ), graph
+                .getNode( "e" ), 2, Direction.OUTGOING, MyRelTypes.R1 );
+               assertNull( findPath.getCost() );
+               assertTrue( findPath.getPaths().isEmpty() );
+
+        findPath = new FindPath( graph.getNode( "a" ), graph
+                .getNode( "e" ), 3, Direction.OUTGOING, MyRelTypes.R1 );
+        assertTrue( findPath.getCost() == 3 );
+        assertTrue( findPath.getPathAsNodes().size() == 4 );
+        assertTrue( findPath.getPathAsRelationships().size() == 3 );
     }
     public void testFindPathChain()
     {
Index: src/main/java/org/neo4j/graphalgo/shortestpath/FindPath.java
===================================================================
--- src/main/java/org/neo4j/graphalgo/shortestpath/FindPath.java        
(revision 3628)
+++ src/main/java/org/neo4j/graphalgo/shortestpath/FindPath.java        
(working copy)
@@ -180,9 +180,9 @@
         dijkstra.setStartNode( startNode );
     }
     
-    public void limitMaxCostToTraverse( MaxCostEvaluator<Integer> evaluator )
+    public void limitMaxCostToTraverse( Integer cost )
     {
-        dijkstra.limitMaxCostToTraverse( evaluator );
+        dijkstra.limitMaxCostToTraverse( cost );
     }
     
     /**
@@ -223,12 +223,6 @@
             RelationshipType... costRelationTypes )
         {
                this(startNode, endNode, relationDirection, costRelationTypes);
-            MaxCostEvaluator<Integer> maxCostComparator = new 
MaxCostEvaluator<Integer>() {
-
-                       public boolean maxCostExceeded(Integer currentCost) {
-                               return currentCost > maxCost;
-                       }
-               };
-               dijkstra.limitMaxCostToTraverse(maxCostComparator);
+               dijkstra.limitMaxCostToTraverse(maxCost);
         }
 }
Index: src/main/java/org/neo4j/graphalgo/shortestpath/Dijkstra.java
===================================================================
--- src/main/java/org/neo4j/graphalgo/shortestpath/Dijkstra.java        
(revision 3628)
+++ src/main/java/org/neo4j/graphalgo/shortestpath/Dijkstra.java        
(working copy)
@@ -65,14 +65,7 @@
     protected long numberOfTraversedRelationShips = 0;
     protected long maxNodesToTraverse = -1;
     protected long numberOfNodesTraversed = 0;
-    protected MaxCostEvaluator<CostType> maxCostEvaluator = new 
MaxCostEvaluator<CostType>()
-    {
-       //this will not limit the path length
-       public boolean maxCostExceeded( CostType currentCost )
-       {
-                       return false;
-               }
-       };
+    protected CostType maxCost = null;
     
     /**
      * @return True if the set limits for the calculation has been reached (but
@@ -92,6 +85,22 @@
         }
         return false;
     }
+    
+    protected boolean limitReached(CostType cost1, CostType cost2)
+    {
+        if ( maxCost != null )
+        {
+            CostType totalCost = costAccumulator.addCosts( cost1, cost2 );
+            if (costComparator.compare( totalCost, maxCost ) > 0)
+            {
+               foundPathsMiddleNodes = null;
+               foundPathsCost = null;
+               return true;
+            }
+        }
+        
+        return false;
+    }
 
     // Result data
     protected boolean doneCalculation = false;
@@ -281,10 +290,6 @@
             {
                 return null;
             }
-            if ( maxCostEvaluator.maxCostExceeded( currentCost ) )
-            {
-                return null;
-            }
             if ( limitReached() )
             {
                 return null;
@@ -478,6 +483,8 @@
             seen1, seen2, dists1, dists2, false );
         DijstraIterator iter2 = new DijstraIterator( endNode, predecessors2,
             seen2, seen1, dists2, dists1, true );
+       Node node1 = null;
+       Node node2 = null;
         while ( iter1.hasNext() && iter2.hasNext() )
         {
             if ( limitReached() )
@@ -486,7 +493,8 @@
             }
             if ( iter1.hasNext() )
             {
-                if ( iter1.next() == null )
+               node1 = iter1.next(); 
+                if ( node1 == null )
                 {
                     break;
                 }
@@ -497,16 +505,22 @@
             }
             if ( !iter1.isDone() && iter2.hasNext() )
             {
-                if ( iter2.next() == null )
+               node2 = iter2.next(); 
+                if ( node2 == null )
                 {
                     break;
                 }
             }
+            if ( limitReached( seen1.get( node1 ), seen2.get( node2 ) ) )
+            {
+               break;
+            }
             if ( iter1.isDone() || iter2.isDone() ) // A path was found
             {
                 return true;
             }
         }
+        
         return false;
     }
 
@@ -778,8 +792,8 @@
      * 
      * @param evaluator The evaluator for 
      */
-    public void limitMaxCostToTraverse( MaxCostEvaluator<CostType> evaluator )
+    public void limitMaxCostToTraverse( CostType maxCost )
     {
-        this.maxCostEvaluator = evaluator;
+        this.maxCost = maxCost;
     }
 }
_______________________________________________
Neo mailing list
User@lists.neo4j.org
https://lists.neo4j.org/mailman/listinfo/user

Reply via email to