switching from vector<state_t> to BV as authoritative input

# Conflicts:
#	src/utility/shortestPaths.h

src/utility/shortestPaths.cpp

@@ -30,20 +30,25 @@ namespace storm {
                 candidatePaths.resize(numStates);
             }
 
+            // extracts the relevant info from the model and delegates to ctor above
+            template <typename T>
+            ShortestPathsGenerator<T>::ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, BitVector const& targetBV)
+                    : ShortestPathsGenerator<T>(model->getTransitionMatrix(), allProbOneMap(targetBV), model->getInitialStates()) {}
+
             // several alternative ways to specify the targets are provided,
             // each converts the targets and delegates to the ctor above
             // I admit it's kind of ugly, but actually pretty convenient (and I've wanted to try C++11 delegation)
             template <typename T>
-            ShortestPathsGenerator<T>::ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model,
+            ShortestPathsGenerator<T>::ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, state_t singleTarget)
-                    state_t singleTarget) : ShortestPathsGenerator<T>(model, std::vector<state_t>{singleTarget}) {}
+                    : ShortestPathsGenerator<T>(model, std::vector<state_t>{singleTarget}) {}
 
             template <typename T>
-            ShortestPathsGenerator<T>::ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model,
+            ShortestPathsGenerator<T>::ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, std::vector<state_t> const& targetList)
-                    storage::BitVector const& targetBV) : ShortestPathsGenerator<T>(model, bitvectorToList(targetBV)) {}
+                    : ShortestPathsGenerator<T>(model, BitVector(model->getNumberOfStates(), targetList)) {}
 
             template <typename T>
-            ShortestPathsGenerator<T>::ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model,
+            ShortestPathsGenerator<T>::ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, std::string const& targetLabel)
-                    std::string const& targetLabel) : ShortestPathsGenerator<T>(model, bitvectorToList(model->getStates(targetLabel))) {}
+                    : ShortestPathsGenerator<T>(model, model->getStates(targetLabel)) {}
 
             template <typename T>
             T ShortestPathsGenerator<T>::getDistance(unsigned long k) {
@@ -72,10 +77,10 @@ namespace storm {
             }
 
             template <typename T>
-            state_list_t ShortestPathsGenerator<T>::getPathAsList(unsigned long k) {
+            std::vector<state_t> ShortestPathsGenerator<T>::getPathAsList(unsigned long k) {
                 computeKSP(k);
 
-                state_list_t backToFrontList;
+                std::vector<state_t> backToFrontList;
 
                 Path<T> currentPath = kShortestPaths[metaTarget][k - 1];
                 boost::optional<state_t> maybePredecessor = currentPath.predecessorNode;

src/utility/shortestPaths.h

@@ -12,21 +12,20 @@
 //       for more information about the purpose, design decisions,
 //       etc., you may consult `shortestPath.md`. - Tom
 
-/*
+// TODO: test whether using BitVector instead of vector<state_t> is
- * TODO:
+//       faster for storing predecessors etc.
- *  - take care of self-loops of target states
- *  - implement target group
- *  - think about how to get paths with new nodes, rather than different
- *    paths over the same set of states (which happens often)
- */
 
+// Now using BitVectors instead of vector<state_t> in the API because
+// BitVectors are used throughout Storm to represent a (unordered) list
+// of states.
+// (Even though both initialStates and targets are probably very sparse.)
 
 namespace storm {
     namespace utility {
         namespace ksp {
             typedef storage::sparse::state_type state_t;
-            typedef std::vector<state_t> state_list_t;
             using BitVector = storage::BitVector;
+            typedef std::vector<state_t> ordered_state_list_t;
 
             template <typename T>
             struct Path {
@@ -57,13 +56,12 @@ namespace storm {
                  * Modifications are done locally, `model` remains unchanged.
                  * Target (group) cannot be changed.
                  */
-                // FIXME: this shared_ptr-passing business might be a bad idea
+                ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, BitVector const& targetBV);
-                ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, state_list_t const& targets);
 
                 // allow alternative ways of specifying the target,
-                // all of which will be converted to list and delegated to constructor above
+                // all of which will be converted to BitVector and delegated to constructor above
                 ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, state_t singleTarget);
-                ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, storage::BitVector const& targetBV);
+                ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, std::vector<state_t> const& targetList);
                 ShortestPathsGenerator(std::shared_ptr<models::sparse::Model<T>> model, std::string const& targetLabel = "target");
 
                 // a further alternative: use transition matrix of maybe-states
@@ -94,7 +92,7 @@ namespace storm {
                  * Computes KSP if not yet computed.
                  * @throws std::invalid_argument if no such k-shortest path exists
                  */
-                state_list_t getPathAsList(unsigned long k);
+                ordered_state_list_t getPathAsList(unsigned long k);
 
 
             private:
@@ -105,9 +103,9 @@ namespace storm {
                 BitVector initialStates;
                 std::unordered_map<state_t, T> targetProbMap;
 
-                std::vector<state_list_t>             graphPredecessors;
+                std::vector<ordered_state_list_t>     graphPredecessors; // FIXME is a switch to BitVector a good idea here?
                 std::vector<boost::optional<state_t>> shortestPathPredecessors;
-                std::vector<state_list_t>             shortestPathSuccessors;
+                std::vector<ordered_state_list_t>     shortestPathSuccessors;
                 std::vector<T>                        shortestPathDistances;
 
                 std::vector<std::vector<Path<T>>> kShortestPaths;

test/functional/utility/KSPTest.cpp

@@ -107,7 +107,7 @@ TEST(KSPTest, kspPathAsList) {
     storm::utility::ksp::ShortestPathsGenerator<double> spg(model, testState);
 
     // TODO: use path that actually has a loop or something to make this more interesting
-    auto reference = std::vector<storm::utility::ksp::state_t>{300, 293, 285, 279, 271, 265, 259, 252, 244, 237, 229, 223, 217, 210, 202, 195, 187, 181, 175, 168, 160, 153, 145, 139, 133, 126, 118, 111, 103, 97, 91, 84, 76, 69, 61, 55, 49, 43, 35, 41, 32, 25, 19, 14, 10, 7, 4, 2, 1, 0};
+    auto reference = storm::utility::ksp::ordered_state_list_t{300, 293, 285, 279, 271, 265, 259, 252, 244, 237, 229, 223, 217, 210, 202, 195, 187, 181, 175, 168, 160, 153, 145, 139, 133, 126, 118, 111, 103, 97, 91, 84, 76, 69, 61, 55, 49, 43, 35, 41, 32, 25, 19, 14, 10, 7, 4, 2, 1, 0};
     auto list = spg.getPathAsList(7);
 
     EXPECT_EQ(list, reference);