Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/dsf/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/dsf/dsf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/dsf/mobility/Itinerary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
9 changes: 6 additions & 3 deletions src/dsf/mobility/Itinerary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
43 changes: 33 additions & 10 deletions src/dsf/mobility/RoadDynamics.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,10 @@
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);

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 17.8 rule Note

MISRA 17.8 rule

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 13.4 rule Note

MISRA 13.4 rule
/// @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)
Expand Down Expand Up @@ -413,7 +416,6 @@
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(),
Expand Down Expand Up @@ -470,12 +472,6 @@

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) {
Expand Down Expand Up @@ -972,7 +968,7 @@
}
++it;
continue;
}

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 15.5 rule Note

MISRA 15.5 rule
if (!m_turnCounts.empty() && pAgent->streetId().has_value()) {
++m_turnCounts[*(pAgent->streetId())][nextStreet->id()];
}
Expand Down Expand Up @@ -1250,12 +1246,39 @@

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()) {

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 14.4 rule Note

MISRA 14.4 rule
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);
Comment on lines +1277 to +1278
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple itineraries can share the same destination node. Unconditionally erasing destination from m_destinationNodes and m_originNodes when removing an itinerary will incorrectly remove these nodes even if other valid itineraries still use them as destinations.

Consider checking if any remaining itineraries use the same destination before removing it from these maps, or maintain reference counts for shared destinations.

Suggested change
m_destinationNodes.erase(destination);
m_originNodes.erase(destination);
// Only erase destination/origin nodes if no other itinerary uses this destination
bool destinationStillUsed = std::any_of(
m_itineraries.begin(), m_itineraries.end(),
[id, destination](const auto& pair) {
return pair.first != id && pair.second->destination() == destination;
});
if (!destinationStillUsed) {
m_destinationNodes.erase(destination);
m_originNodes.erase(destination);
}

Copilot uses AI. Check for mistakes.
m_itineraries.erase(id);
}
}
spdlog::debug("End updating paths.");
}

Expand Down
25 changes: 23 additions & 2 deletions test/mobility/Test_dynamics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,27 @@
}
}
}
GIVEN("A disconnected graph") {
Street s1{0, std::make_pair(0, 1), 10.};

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 12.3 rule Note test

MISRA 12.3 rule
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") {
Expand Down Expand Up @@ -549,7 +570,7 @@
}
}
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.};
Expand Down Expand Up @@ -586,7 +607,7 @@
CHECK_EQ(dynamics.nAgents(), 0);
}
}
spdlog::set_level(spdlog::level::info);
// spdlog::set_level(spdlog::level::info);
}
}
SUBCASE("TrafficLights") {
Expand Down
Loading