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
9 changes: 9 additions & 0 deletions src/dsf/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ PYBIND11_MODULE(dsf_cpp, m) {
.def("autoMapStreetLanes",
&dsf::mobility::RoadNetwork::autoMapStreetLanes,
dsf::g_docstrings.at("dsf::mobility::RoadNetwork::autoMapStreetLanes").c_str())
.def("setStreetStationaryWeights",
&dsf::mobility::RoadNetwork::setStreetStationaryWeights,
pybind11::arg("weights"),
dsf::g_docstrings.at("dsf::mobility::RoadNetwork::setStreetStationaryWeights")
.c_str())
Comment on lines +175 to +179
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The Python bindings reference dsf::mobility::RoadNetwork::setStreetStationaryWeights in the docstrings map (line 178), but this method has no documentation in RoadNetwork.hpp (line 194). This will cause a runtime error when loading the Python module, as the docstrings are generated from Doxygen comments and this key won't exist in the g_docstrings map. Add proper Doxygen documentation for this public API method.

Copilot uses AI. Check for mistakes.
.def(
"importEdges",
[](dsf::mobility::RoadNetwork& self, const std::string& fileName) {
Expand Down Expand Up @@ -531,6 +536,10 @@ PYBIND11_MODULE(dsf_cpp, m) {
pybind11::arg("ratio") = 1.3,
dsf::g_docstrings.at("dsf::mobility::RoadDynamics::optimizeTrafficLights")
.c_str())
.def("graph",
&dsf::mobility::FirstOrderDynamics::graph,
pybind11::return_value_policy::reference_internal,
dsf::g_docstrings.at("dsf::Dynamics::graph").c_str())
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The Python bindings reference dsf::Dynamics::graph in the docstrings map (line 542), but there's no documentation for a non-const graph() method in the Dynamics class. The base Dynamics class only has a const version (inline const auto& graph() const). This will cause a runtime error when the docstrings are loaded, as the key won't exist in the generated docstrings map. Either:

  1. Add documentation for the non-const graph() method if it exists
  2. Use the correct docstring key if this method has different documentation
  3. Remove the docstring lookup if this is meant to reuse the const version's documentation
Suggested change
dsf::g_docstrings.at("dsf::Dynamics::graph").c_str())
dsf::g_docstrings.at("dsf::Dynamics::graph() const").c_str())

Copilot uses AI. Check for mistakes.
.def("nAgents",
&dsf::mobility::FirstOrderDynamics::nAgents,
dsf::g_docstrings.at("dsf::mobility::RoadDynamics::nAgents").c_str())
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 = 9;
static constexpr uint8_t DSF_VERSION_PATCH = 10;

static auto const DSF_VERSION =
std::format("{}.{}.{}", DSF_VERSION_MAJOR, DSF_VERSION_MINOR, DSF_VERSION_PATCH);
Expand Down
7 changes: 6 additions & 1 deletion src/dsf/mobility/RoadDynamics.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -497,11 +497,13 @@ namespace dsf::mobility {
std::optional<Id> previousNodeId = std::nullopt;
std::set<Id> forbiddenTurns;
double speedCurrent = 1.0;
double stationaryWeightCurrent = 1.0;
if (pAgent->streetId().has_value()) {
auto const& pStreetCurrent{this->graph().edge(pAgent->streetId().value())};
previousNodeId = pStreetCurrent->source();
forbiddenTurns = pStreetCurrent->forbiddenTurns();
speedCurrent = pStreetCurrent->maxSpeed();
stationaryWeightCurrent = pStreetCurrent->stationaryWeight();
}

// Get path targets for non-random agents
Expand Down Expand Up @@ -550,7 +552,10 @@ namespace dsf::mobility {

// Calculate base probability
auto const speedNext{pStreetOut->maxSpeed()};
double probability = speedCurrent * speedNext;
double const stationaryWeightNext = pStreetOut->stationaryWeight();
auto const weightRatio{stationaryWeightNext /
stationaryWeightCurrent}; // SQRT (p_i / p_j)
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The comment incorrectly states "SQRT (p_i / p_j)" but the actual calculation uses sqrt(weightRatio) where weightRatio = stationaryWeightNext / stationaryWeightCurrent. This suggests the comment should either say "sqrt(p_next / p_current)" or clarify what p_i and p_j represent in this context. The indices i and j are ambiguous here.

Suggested change
stationaryWeightCurrent}; // SQRT (p_i / p_j)
stationaryWeightCurrent}; // sqrt(p_next / p_current)

Copilot uses AI. Check for mistakes.
double probability = speedCurrent * speedNext * std::sqrt(weightRatio);

// Apply error probability for non-random agents
if (this->m_errorProbability.has_value() && !pathTargets.empty()) {
Expand Down
28 changes: 16 additions & 12 deletions src/dsf/mobility/RoadNetwork.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,18 +263,6 @@ namespace dsf::mobility {
addCoil(edge_id);
}
}
// Check for transition probabilities
// if (!edge_properties.at_key("transition_probabilities").error() &&
// edge_properties["transition_probabilities"].has_value()) {
// auto const& tp = edge_properties["transition_probabilities"];
// std::unordered_map<Id, double> transitionProbabilities;
// for (auto const& [key, value] : tp.get_object()) {
// auto const targetStreetId = static_cast<Id>(std::stoull(std::string(key)));
// auto const probability = static_cast<double>(value.get_double());
// transitionProbabilities.emplace(targetStreetId, probability);
// }
// edge(edge_id)->setTransitionProbabilities(transitionProbabilities);
// }
}
this->m_nodes.rehash(0);
this->m_edges.rehash(0);
Expand Down Expand Up @@ -843,6 +831,22 @@ namespace dsf::mobility {
addEdge<Street>(std::move(street));
}

void RoadNetwork::setStreetStationaryWeights(
std::unordered_map<Id, double> const& weights) {
std::for_each(DSF_EXECUTION m_edges.cbegin(),
m_edges.cend(),
[this, &weights](auto const& pair) {
auto const streetId = pair.first;
auto const& pStreet = pair.second;
auto it = weights.find(streetId);
if (it != weights.end()) {
pStreet->setStationaryWeight(it->second);
} else {
pStreet->setStationaryWeight(1.0);
}
});
}

const std::unique_ptr<Street>* RoadNetwork::street(Id source, Id destination) const {
// Get the iterator at id m_cantorPairingHashing(source, destination)
try {
Expand Down
9 changes: 9 additions & 0 deletions src/dsf/mobility/RoadNetwork.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ namespace dsf::mobility {
/// @brief Construct a new RoadNetwork object
/// @param adj An adjacency matrix made by a SparseMatrix representing the graph's adjacency matrix
RoadNetwork(AdjacencyMatrix const& adj);
// Disable copy constructor and copy assignment operator
RoadNetwork(const RoadNetwork&) = delete;
RoadNetwork& operator=(const RoadNetwork&) = delete;
// Enable move constructor and move assignment operator
RoadNetwork(RoadNetwork&&) = default;
RoadNetwork& operator=(RoadNetwork&&) = default;

/// @brief Get the graph's number of coil streets
/// @return The number of coil streets
Expand Down Expand Up @@ -184,6 +190,9 @@ namespace dsf::mobility {
requires is_street_v<std::remove_reference_t<T1>> &&
(is_street_v<std::remove_reference_t<Tn>> && ...)
void addStreets(T1&& street, Tn&&... streets);
/// @brief Set the streets' stationary weights
/// @param streetWeights A map where the key is the street id and the value is the street stationary weight. If a street id is not present in the map, its stationary weight is set to 1.0.
void setStreetStationaryWeights(std::unordered_map<Id, double> const& streetWeights);

/// @brief Get a street from the graph
/// @param source The source node
Expand Down
21 changes: 10 additions & 11 deletions src/dsf/mobility/Street.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@
AgentComparator>
m_movingAgents;
std::vector<Direction> m_laneMapping;
// std::unordered_map<Id, double> m_transitionProbabilities;
std::optional<Counter> m_counter;
double m_stationaryWeight{1.0};

public:
/// @brief Construct a new Street object
Expand Down Expand Up @@ -84,12 +84,12 @@
/// @param meanVehicleLength The mean vehicle length
/// @throw std::invalid_argument If the mean vehicle length is negative
static void setMeanVehicleLength(double meanVehicleLength);
// /// @brief Set the street's transition probabilities
// /// @param transitionProbabilities The street's transition probabilities
// inline void setTransitionProbabilities(
// std::unordered_map<Id, double> const& transitionProbabilities) {
// m_transitionProbabilities = transitionProbabilities;
// };
/// @brief Set the street's stationary weight
/// @param weight The street's stationary weight
inline void setStationaryWeight(double const weight) {
weight > 0. ? m_stationaryWeight = weight

Check warning

Code scanning / Cppcheck (reported by Codacy)

AST broken, ternary operator missing operand(s) Warning

AST broken, ternary operator missing operand(s)
: throw std::invalid_argument("Stationary weight must be positive");
}
/// @brief Enable a coil (dsf::Counter sensor) on the street
/// @param name The name of the counter (default is "Coil_<street_id>")
void enableCounter(std::string name = std::string());
Expand Down Expand Up @@ -117,10 +117,9 @@
/// @brief Check if the street is full
/// @return bool, True if the street is full, false otherwise
inline bool isFull() const final { return this->nAgents() == this->m_capacity; }

// inline auto const& transitionProbabilities() const {
// return m_transitionProbabilities;
// }
/// @brief Get the street's stationary weight
/// @return double The street's stationary weight
inline auto stationaryWeight() const noexcept { return m_stationaryWeight; }
/// @brief Get the name of the counter
/// @return std::string The name of the counter
inline auto counterName() const {
Expand Down
99 changes: 99 additions & 0 deletions test/mobility/Test_dynamics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "dsf/mobility/RoadNetwork.hpp"
#include "dsf/mobility/Itinerary.hpp"
#include "dsf/mobility/Street.hpp"
#include "dsf/mobility/Intersection.hpp"

#include <chrono>
#include <cstdint>
Expand Down Expand Up @@ -1282,3 +1283,101 @@
}
}
}

TEST_CASE("Stationary Weights Impact on Random Navigation") {
RoadNetwork network;

network.addNode<Intersection>(0);
network.addNode<Intersection>(1);
network.addNode<Intersection>(2);
network.addNode<Intersection>(3);

const int numAgents = 3000;
const int highCapacity = numAgents + 100;

// Street 0: 0 -> 1
// High length to hold all agents, high transport capacity to move them all at once
network.addStreet(Street(0,
{0, 1},

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 12.3 rule Note test

MISRA 12.3 rule
100.0,
10.0,
1,
"Street 0",
{},
highCapacity,
static_cast<double>(highCapacity)));
// Street 1: 1 -> 2
network.addStreet(Street(1,
{1, 2},

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 12.3 rule Note test

MISRA 12.3 rule
100.0,
10.0,
1,
"Street 1",
{},
highCapacity,
static_cast<double>(highCapacity)));
// Street 2: 1 -> 3
network.addStreet(Street(2,
{1, 3},

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 12.3 rule Note test

MISRA 12.3 rule
100.0,
10.0,
1,
"Street 2",
{},
highCapacity,
static_cast<double>(highCapacity)));

// Set stationary weights
// Street 0: weight 1.0
network.edge(0)->setStationaryWeight(1.0);

// Street 1: weight 1.0
network.edge(1)->setStationaryWeight(1.0);

// Street 2: weight 4.0
network.edge(2)->setStationaryWeight(4.0);

// Adjust node capacities to match street capacities
network.adjustNodeCapacities();

// Initialize dynamics
FirstOrderDynamics dynamics(network, false, 42, 0.0);

// Add many random agents to Street 0
for (int i = 0; i < numAgents; ++i) {
auto agent = std::make_unique<Agent>(0, std::nullopt, 0);
agent->setStreetId(0);
agent->setSpeed(10.0);
agent->setFreeTime(0);
dynamics.graph().edge(0)->addAgent(std::move(agent));
}

// Evolve simulation
// Step 1: Agents move from Street 0 to Node 1
dynamics.evolve();

// Step 2: Agents move from Node 1 to Street 1 or 2
dynamics.evolve();

// Count agents on Street 1 and Street 2
size_t countStreet1 = dynamics.graph().edge(1)->nAgents();
size_t countStreet2 = dynamics.graph().edge(2)->nAgents();

// Expected probabilities:
// P(1) ~ speed * speed * sqrt(1/1) = 100
// P(2) ~ speed * speed * sqrt(4/1) = 200
Comment on lines +1367 to +1368
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The test comment contains an error in the probability calculation. The comment states:

// P(1) ~ speed * speed * sqrt(1/1) = 100
// P(2) ~ speed * speed * sqrt(4/1) = 200

However, based on the actual formula in RoadDynamics.hpp line 558 (probability = speedCurrent * speedNext * std::sqrt(weightRatio)), the calculation should be:

  • P(1) = speedCurrent (10.0) × speedNext (10.0) × sqrt(1.0/1.0) = 100
  • P(2) = speedCurrent (10.0) × speedNext (10.0) × sqrt(4.0/1.0) = 200

The formulas are actually correct, but stating "speed * speed" is misleading since it's actually speedCurrent × speedNext (which happen to be the same value in this test). Consider clarifying this as "speedCurrent * speedNext" for accuracy.

Suggested change
// P(1) ~ speed * speed * sqrt(1/1) = 100
// P(2) ~ speed * speed * sqrt(4/1) = 200
// P(1) ~ speedCurrent * speedNext * sqrt(1/1) = 10.0 * 10.0 * 1 = 100
// P(2) ~ speedCurrent * speedNext * sqrt(4/1) = 10.0 * 10.0 * 2 = 200

Copilot uses AI. Check for mistakes.
// Ratio 1:2 -> P(1) = 1/3, P(2) = 2/3

double ratio =
(countStreet1 > 0) ? static_cast<double>(countStreet2) / countStreet1 : 0.0;

Check notice

Code scanning / Cppcheck (reported by Codacy)

MISRA 10.4 rule Note test

MISRA 10.4 rule

std::cout << "Agents on Street 1: " << countStreet1 << std::endl;
std::cout << "Agents on Street 2: " << countStreet2 << std::endl;
std::cout << "Ratio (Street 2 / Street 1): " << ratio << std::endl;

Comment on lines +1374 to +1377
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Debug output statements (std::cout) should be removed from test code or replaced with proper logging. These print statements were likely used during development and should not remain in production test code as they clutter test output.

Suggested change
std::cout << "Agents on Street 1: " << countStreet1 << std::endl;
std::cout << "Agents on Street 2: " << countStreet2 << std::endl;
std::cout << "Ratio (Street 2 / Street 1): " << ratio << std::endl;

Copilot uses AI. Check for mistakes.
// Check if ratio is close to 2.0
CHECK_EQ(ratio, doctest::Approx(2.0).epsilon(0.1)); // Allow 10% error margin

// Check total agents preserved (some might be in transit or node if something went wrong)
CHECK_EQ(countStreet1 + countStreet2, numAgents);
}
Loading