Merge pull request #374 from physycom/pathThrows

Add `throw_on_empty` param to updatePaths

Author: Grufoony

src/dsf/bindings.cpp

@@ -475,6 +475,7 @@ PYBIND11_MODULE(dsf_cpp, m) {
            dsf::g_docstrings.at("dsf::mobility::RoadDynamics::initTurnCounts").c_str())
       .def("updatePaths",
            &dsf::mobility::FirstOrderDynamics::updatePaths,
+           pybind11::arg("throw_on_empty") = true,
            dsf::g_docstrings.at("dsf::mobility::RoadDynamics::updatePaths").c_str())
       .def(
           "addAgentsUniformly",

src/dsf/dsf.hpp

@@ -6,7 +6,7 @@
 
 static constexpr uint8_t DSF_VERSION_MAJOR = 4;
 static constexpr uint8_t DSF_VERSION_MINOR = 4;
-static constexpr uint8_t DSF_VERSION_PATCH = 8;
+static constexpr uint8_t DSF_VERSION_PATCH = 9;
 
 static auto const DSF_VERSION =
     std::format("{}.{}.{}", DSF_VERSION_MAJOR, DSF_VERSION_MINOR, DSF_VERSION_PATCH);

src/dsf/mobility/Itinerary.cpp

@@ -36,9 +36,6 @@ namespace dsf::mobility {
     m_path = std::move(pathCollection);
   }
 
-  Id Itinerary::id() const { return m_id; }
-  Id Itinerary::destination() const { return m_destination; }
-  PathCollection const& Itinerary::path() const { return m_path; }
   void Itinerary::save(const std::string& fileName) const {
     // Open binary file
     std::ofstream outFile{fileName, std::ios::binary};

src/dsf/mobility/Itinerary.hpp

@@ -48,13 +48,16 @@ namespace dsf::mobility {
 
     /// @brief Get the itinerary's id
     /// @return Id, The itinerary's id
-    Id id() const;
+    inline auto id() const noexcept { return m_id; };
     /// @brief Get the itinerary's destination
     /// @return Id, The itinerary's destination
-    Id destination() const;
+    inline auto destination() const noexcept { return m_destination; };
     /// @brief Get the itinerary's path
     /// @return PathCollection const&, The itinerary's path
-    PathCollection const& path() const;
+    inline auto const& path() const noexcept { return m_path; };
+    /// @brief Check if the itinerary's path is empty
+    /// @return true if the itinerary's path is empty, false otherwise
+    inline auto empty() const noexcept { return m_path.empty(); };
     /// @brief Save the itinerary to a binary file
     /// @param fileName The name of the file to save the itinerary to
     void save(const std::string& fileName) const;

src/dsf/mobility/RoadDynamics.hpp

@@ -191,7 +191,10 @@ namespace dsf::mobility {
     void resetTurnCounts();
 
     /// @brief Update the paths of the itineraries based on the given weight function
-    void updatePaths();
+    /// @param throw_on_empty If true, throws an exception if an itinerary has an empty path (default is true)
+    /// If false, removes the itinerary with empty paths and the associated node from the origin/destination nodes
+    /// @throws std::runtime_error if throw_on_empty is true and an itinerary has an empty path
+    void updatePaths(bool const throw_on_empty = true);
     /// @brief Add agents uniformly on the road network
     /// @param nAgents The number of agents to add
     /// @param itineraryId The id of the itinerary to use (default is std::nullopt)
@@ -413,7 +416,6 @@ namespace dsf::mobility {
     for (auto const& [nodeId, weight] : this->m_destinationNodes) {
       m_itineraries.emplace(nodeId, std::make_unique<Itinerary>(nodeId, nodeId));
     }
-    // updatePaths();
     std::for_each(
         this->graph().edges().cbegin(),
         this->graph().edges().cend(),
@@ -470,12 +472,6 @@ namespace dsf::mobility {
 
     auto const& path{this->graph().allPathsTo(
         pItinerary->destination(), m_weightFunction, m_weightTreshold)};
-    if (path.empty()) {
-      throw std::runtime_error(
-          std::format("No path found for itinerary {} with destination node {}",
-                      pItinerary->id(),
-                      pItinerary->destination()));
-    }
     pItinerary->setPath(path);
     auto const newSize{pItinerary->path().size()};
     if (oldSize > 0 && newSize != oldSize) {
@@ -1250,12 +1246,39 @@ namespace dsf::mobility {
 
   template <typename delay_t>
     requires(is_numeric_v<delay_t>)
-  void RoadDynamics<delay_t>::updatePaths() {
+  void RoadDynamics<delay_t>::updatePaths(bool const throw_on_empty) {
     spdlog::debug("Init updating paths...");
+    tbb::concurrent_vector<Id> emptyItineraries;
     tbb::parallel_for_each(
         this->itineraries().cbegin(),
         this->itineraries().cend(),
-        [this](auto const& pair) -> void { this->m_updatePath(pair.second); });
+        [this, throw_on_empty, &emptyItineraries](auto const& pair) -> void {
+          auto const& pItinerary{pair.second};
+          this->m_updatePath(pItinerary);
+          if (pItinerary->empty()) {
+            if (!throw_on_empty) {
+              spdlog::warn("No path found for itinerary {} with destination node {}",
+                           pItinerary->id(),
+                           pItinerary->destination());
+              emptyItineraries.push_back(pItinerary->id());
+              return;
+            }
+            throw std::runtime_error(
+                std::format("No path found for itinerary {} with destination node {}",
+                            pItinerary->id(),
+                            pItinerary->destination()));
+          }
+        });
+    if (!emptyItineraries.empty()) {
+      spdlog::warn("Removing {} itineraries with no valid path from the dynamics.",
+                   emptyItineraries.size());
+      for (auto const& id : emptyItineraries) {
+        auto const destination = m_itineraries.at(id)->destination();
+        m_destinationNodes.erase(destination);
+        m_originNodes.erase(destination);
+        m_itineraries.erase(id);
+      }
+    }
     spdlog::debug("End updating paths.");
   }
 

test/mobility/Test_dynamics.cpp

@@ -425,6 +425,27 @@ TEST_CASE("FirstOrderDynamics") {
         }
       }
     }
+    GIVEN("A disconnected graph") {
+      Street s1{0, std::make_pair(0, 1), 10.};
+      RoadNetwork graph;
+      graph.addStreets(s1);
+      FirstOrderDynamics dynamics{graph, false, 69, 0., dsf::PathWeight::LENGTH};
+
+      WHEN(
+          "We add an impossible itinerary (to source node) and update paths with "
+          "throw_on_empty=true") {
+        dynamics.addItinerary(std::unique_ptr<Itinerary>(new Itinerary(0, 0)));
+        THEN("It throws an exception") { CHECK_THROWS(dynamics.updatePaths(true)); }
+      }
+
+      WHEN(
+          "We add an impossible itinerary (to source node) and update paths with "
+          "throw_on_empty=false") {
+        dynamics.addItinerary(std::unique_ptr<Itinerary>(new Itinerary(0, 0)));
+        dynamics.updatePaths(false);
+        THEN("The itinerary is removed") { CHECK(dynamics.itineraries().empty()); }
+      }
+    }
   }
   SUBCASE("Evolve") {
     GIVEN("A dynamics object and an itinerary") {
@@ -549,7 +570,7 @@ TEST_CASE("FirstOrderDynamics") {
       }
     }
     GIVEN("A simple network and an agent with forced itinerary") {
-      spdlog::set_level(spdlog::level::trace);
+      // spdlog::set_level(spdlog::level::trace);
       Street s0_1{1, std::make_pair(0, 1), 30., 15.};
       Street s1_0{3, std::make_pair(1, 0), 30., 15.};
       Street s1_2{5, std::make_pair(1, 2), 30., 15.};
@@ -586,7 +607,7 @@ TEST_CASE("FirstOrderDynamics") {
           CHECK_EQ(dynamics.nAgents(), 0);
         }
       }
-      spdlog::set_level(spdlog::level::info);
+      // spdlog::set_level(spdlog::level::info);
     }
   }
   SUBCASE("TrafficLights") {