Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
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 @@
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 @@
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
8 changes: 8 additions & 0 deletions src/dsf/mobility/RoadNetwork.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@
/// @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 @@ -185,6 +191,8 @@
(is_street_v<std::remove_reference_t<Tn>> && ...)
void addStreets(T1&& street, Tn&&... streets);

void setStreetStationaryWeights(std::unordered_map<Id, double> const& streetWeights);

/// @brief Get a street from the graph
/// @param source The source node
/// @param destination The destination node
Expand Down
18 changes: 7 additions & 11 deletions src/dsf/mobility/Street.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ namespace dsf::mobility {
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,9 @@ namespace dsf::mobility {
/// @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) { m_stationaryWeight = weight; }
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.

Missing input validation for the weight parameter. Negative or zero weights could lead to incorrect probability calculations or mathematical errors (e.g., when taking square roots in RoadDynamics.hpp line 558). Consider adding validation to ensure weights are positive, or document if non-positive values are intentionally allowed.

Suggested change
inline void setStationaryWeight(double const weight) { m_stationaryWeight = weight; }
inline void setStationaryWeight(double const weight) {
if (weight <= 0.0) {
throw std::invalid_argument("Stationary weight must be positive.");
}
m_stationaryWeight = weight;
}

Copilot uses AI. Check for mistakes.
/// @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 +114,9 @@ namespace dsf::mobility {
/// @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
Loading