Add counting on OD nodes

[Grufoony]

No description provided.

[Copilot]

Pull request overview

This pull request adds functionality to track the count of agents at their origin and destination nodes in the RoadDynamics class. The implementation uses thread-safe concurrent containers to support counting in parallel simulation contexts.

Changes:

• Added m_originCounts and m_destinationCounts concurrent maps to track OD node counts
• Implemented originCounts() and destinationCounts() methods with optional reset functionality
• Added Python bindings for the new counting methods

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
src/dsf/mobility/RoadDynamics.hpp	Added concurrent maps for OD counting, implemented counting logic in addAgent/m_killAgent, and added getter methods with reset capability
src/dsf/bindings.cpp	Added Python bindings for originCounts and destinationCounts methods with conversion to Python dict

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The contains() check followed by operator[] is not thread-safe, even with concurrent_unordered_map. Between checking contains() and incrementing the value, another thread could insert the same key, or the operation is not atomic. Consider using try_emplace() or insert() which are atomic operations, or use the try_emplace pattern to avoid the race condition.

Suggested change

	if (m_originCounts.contains(*optNodeId)) [[likely]] {
	++m_originCounts[*optNodeId];
	} else [[unlikely]] {
	m_originCounts[*optNodeId] = 1;
	}
	}
	}
	auto [it, inserted] = m_originCounts.insert({*optNodeId, std::size_t{1}});
	if (!inserted) [[likely]] {
	++(it->second);
	}
	}
	}
	}

[Copilot]

The contains() check followed by operator[] is not thread-safe, even with concurrent_unordered_map. Since m_killAgent is called from m_evolveStreet which runs in parallel (tbb::parallel_for), multiple threads could be accessing this map concurrently. Between checking contains() and incrementing the value, another thread could insert the same key, or the increment operation might not be atomic. Consider using try_emplace() or insert() which are atomic operations, or use the try_emplace pattern to avoid the race condition.

Suggested change

	if (m_destinationCounts.contains(pNode->id())) [[likely]] {
	++m_destinationCounts[pNode->id()];
	} else [[unlikely]] {
	m_destinationCounts[pNode->id()] = 1;
	}
	auto [it, inserted] =
	m_destinationCounts.try_emplace(pNode->id(), static_cast<std::size_t>(0));
	++(it->second);

[Copilot]

The move-and-clear pattern for reset is not thread-safe when the map can be accessed concurrently. If reset=true, another thread could be incrementing values during the move operation or immediately after. Consider using a mutex to protect the reset operation, or document that these methods should only be called when no other threads are actively modifying the maps (e.g., between simulation steps).

[Copilot]

The move-and-clear pattern for reset is not thread-safe when the map can be accessed concurrently. If reset=true, another thread could be incrementing values during the move operation or immediately after. Consider using a mutex to protect the reset operation, or document that these methods should only be called when no other threads are actively modifying the maps (e.g., between simulation steps).

[Copilot]

The new originCounts and destinationCounts functionality lacks test coverage. Since the test suite in test/mobility/Test_dynamics.cpp contains comprehensive tests for other features, consider adding test cases that verify: 1) agents are correctly counted at their origin nodes when added, 2) agents are correctly counted at their destination nodes when they arrive, 3) the reset parameter works correctly, and 4) the counting works correctly under concurrent access during parallel evolve operations.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.09%. Comparing base (eb83c88) to head (889e3d8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #392      +/-   ##
==========================================
+ Coverage   84.76%   85.09%   +0.32%     
==========================================
  Files          53       53              
  Lines        5699     5763      +64     
  Branches      640      649       +9     
==========================================
+ Hits         4831     4904      +73     
+ Misses        857      848       -9     
  Partials       11       11              

Flag	Coverage Δ
unittests	85.09% <100.00%> (+0.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.