Merge pull request #36 from clue-labs/fix-tsp

Fix setting upper limit for TSP bruteforce via MST algorithm

src/TravelingSalesmanProblem/Base.php

@@ -2,21 +2,21 @@
 
 namespace Graphp\Algorithms\TravelingSalesmanProblem;
 
-use Fhaculty\Graph\Walk;
-use Fhaculty\Graph\Vertex;
-use Fhaculty\Graph\Edge\Base as Edge;
+use Fhaculty\Graph\Graph;
 use Fhaculty\Graph\Set\Edges;
+use Fhaculty\Graph\Vertex;
+use Fhaculty\Graph\Walk;
 use Graphp\Algorithms\Base as AlgorithmBase;
 
 abstract class Base extends AlgorithmBase
 {
     /**
      * get resulting graph with the (first) best circle of edges connecting all vertices
      *
-     * @throws Exception on error
+     * @throws \Exception on error
      * @return Graph
-     * @uses Tsp::getGraph()
-     * @uses AlgorithmTsp::getEdges()
+     * @uses self::getGraph()
+     * @uses self::getEdges()
      * @uses Graph::createGraphCloneEdges()
      */
     public function createGraph()
@@ -42,8 +42,8 @@ abstract protected function getVertexStart();
      * get (first) best circle connecting all vertices
      *
      * @return Walk
-     * @uses AlgorithmTsp::getEdges()
-     * @uses AlgorithmTsp::getVertexStart()
+     * @uses self::getEdges()
+     * @uses self::getVertexStart()
      * @uses Walk::factoryCycleFromEdges()
      */
     public function getCycle()

src/TravelingSalesmanProblem/Bruteforce.php

@@ -2,13 +2,12 @@
 
 namespace Graphp\Algorithms\TravelingSalesmanProblem;
 
+use Fhaculty\Graph\Edge\Base as Edge;
 use Fhaculty\Graph\Exception\UnexpectedValueException;
-
 use Fhaculty\Graph\Exception\UnderflowException;
-
 use Fhaculty\Graph\Graph;
-use Fhaculty\Graph\Vertex;
 use Fhaculty\Graph\Set\Edges;
+use Fhaculty\Graph\Vertex;
 use Graphp\Algorithms\TravelingSalesmanProblem\MinimumSpanningTree as AlgorithmTspMst;
 
 class Bruteforce extends Base
@@ -44,7 +43,7 @@ class Bruteforce extends Base
      * upper limit to use for branch-and-bound (BNB)
      *
      * @var float|NULL
-     * @see AlgorithmTspBruteforce::setUpperLimit()
+     * @see self::setUpperLimit()
      */
     private $upperLimit = NULL;
 
@@ -72,8 +71,8 @@ public function __construct(Graph $graph)
      * this method can be used to optimize the algorithm by providing an upper
      * bound of when to stop branching any further.
      *
-     * @param  double                 $limit
-     * @return AlgorithmTspBruteforce $this (chainable)
+     * @param  double $limit
+     * @return self $this (chainable)
      */
     public function setUpperLimit($limit)
     {
@@ -82,12 +81,18 @@ public function setUpperLimit($limit)
         return $this;
     }
 
+    /**
+     * automatically sets upper limit to use for branch-and-bound from the MST heuristic
+     *
+     * @return self $this (chainable)
+     * @uses AlgorithmTspMst
+     */
     public function setUpperLimitMst()
     {
         $alg = new AlgorithmTspMst($this->graph);
-        $limit = $alg->createGraph()->getWeight();
+        $this->upperLimit = $alg->getWeight();
 
-        return $this->setUpperLimit($limit);
+        return $this;
     }
 
     protected function getVertexStart()
@@ -104,7 +109,7 @@ protected function getGraph()
     /**
      * get resulting (first) best circle of edges connecting all vertices
      *
-     * @throws Exception on error
+     * @throws \Exception on error
      * @return Edges
      */
     public function getEdges()

tests/TravelingSalesmanProblem/BruteforceTest.php

@@ -0,0 +1,41 @@
+<?php
+
+use Fhaculty\Graph\Graph;
+use Graphp\Algorithms\TravelingSalesmanProblem\Bruteforce;
+
+class BruteforceTest extends TestCase
+{
+    public function testGetWeightReturnsExpectedWeightForSimpleCycle()
+    {
+        $graph = new Graph();
+        $a = $graph->createVertex();
+        $b = $graph->createVertex();
+        $c = $graph->createVertex();
+        $a->createEdgeTo($b)->setWeight(1);
+        $b->createEdgeTo($c)->setWeight(2);
+        $c->createEdgeTo($a)->setWeight(3);
+
+        $alg = new Bruteforce($graph);
+
+        $this->assertEquals(6, $alg->getWeight());
+    }
+
+    public function testSetUpperLimitMstSetsExactLimitForSimpleCycle()
+    {
+        $graph = new Graph();
+        $a = $graph->createVertex();
+        $b = $graph->createVertex();
+        $c = $graph->createVertex();
+        $a->createEdgeTo($b)->setWeight(1);
+        $b->createEdgeTo($c)->setWeight(2);
+        $c->createEdgeTo($a)->setWeight(3);
+
+        $alg = new Bruteforce($graph);
+        $alg->setUpperLimitMst();
+
+        $ref = new ReflectionProperty($alg, 'upperLimit');
+        $ref->setAccessible(true);
+
+        $this->assertEquals(6, $ref->getValue($alg));
+    }
+}