Refactor coils #364

Grufoony · 2025-11-06T16:22:10Z

Close Add Spire<...> #110
Refactor Counter class to have only one counter
Streets with a counter sensor are no more a derived class but they have an optional parameter

codecov · 2025-11-06T16:25:03Z

Codecov Report

❌ Patch coverage is 84.61538% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.91%. Comparing base (5a84612) to head (3c03a8d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/dsf/mobility/RoadNetwork.cpp	57.14%	6 Missing ⚠️
src/dsf/mobility/Street.cpp	84.61%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
- Coverage   82.97%   82.91%   -0.07%     
==========================================
  Files          50       50              
  Lines        5057     4892     -165     
  Branches      573      559      -14     
==========================================
- Hits         4196     4056     -140     
+ Misses        853      828      -25     
  Partials        8        8

Flag	Coverage Δ
unittests	`82.91% <84.61%> (-0.07%)`	⬇️

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.

benchmark/Bench_Street.cpp


-static void BM_SpireStreet_AddAgent(benchmark::State& state) {
-  dsf::mobility::Street baseStreet(0,
+static void BM_CoilStreet_AddAgent(benchmark::State& state) {


benchmark/Bench_Street.cpp


-static void BM_SpireStreet_MeanFlow(benchmark::State& state) {
-  dsf::mobility::Street baseStreet(0,
+static void BM_CoilStreet_MeanFlow(benchmark::State& state) {


benchmark/Bench_Street.cpp


-static void BM_SpireStreet_Dequeue(benchmark::State& state) {
-  dsf::mobility::Street baseStreet(0,
+static void BM_CoilStreet_Dequeue(benchmark::State& state) {


src/dsf/mobility/RoadDynamics.hpp

-        if (pStreet->isStochastic()) {
-          value = dynamic_cast<StochasticSpireStreet&>(*pStreet).inputCounts(reset);
-        } else {
-          value = dynamic_cast<SpireStreet&>(*pStreet).inputCounts(reset);


src/dsf/mobility/RoadDynamics.hpp

src/dsf/mobility/Street.cpp

    m_exitQueues[index] = std::move(queue);
  }
+  void Street::enableCounter(std::string name) {
+    if (m_counter.has_value()) {


src/dsf/mobility/Street.cpp

+          std::format("{} already has a counter named {}", *this, m_counter->name()));
+    }
+    if (name.empty()) {
+      name = std::format("Coil_{}", m_id);


src/dsf/mobility/Street.cpp

    m_exitQueues[queueId].push(
        std::move(const_cast<std::unique_ptr<Agent>&>(m_movingAgents.top())));
    m_movingAgents.pop();
+    if (m_counter.has_value()) {


src/dsf/mobility/Street.hpp

    /// @throw std::invalid_argument If the mean vehicle length is negative
    static void setMeanVehicleLength(double meanVehicleLength);

+    void enableCounter(std::string name = std::string());


src/dsf/mobility/Street.hpp

    /// @throw std::invalid_argument If the mean vehicle length is negative
    static void setMeanVehicleLength(double meanVehicleLength);

+    void enableCounter(std::string name = std::string());


github-advanced-security

Cppcheck (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copilot

Pull Request Overview

This PR refactors the traffic counting system by replacing the SpireStreet and StochasticSpireStreet classes with an optional counter feature that can be enabled on any Street. The PR simplifies the counter implementation from tracking separate input/output flows to a single count of vehicles passing through.

Removes SpireStreet and StochasticSpireStreet inheritance-based classes
Adds an optional Counter member to the base Street class
Simplifies Counter class to track a single count value instead of separate input/output
Updates all examples, tests, and benchmarks to use the new API

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/dsf/mobility/Street.hpp	Adds optional counter member, removes SpireStreet classes, adds enableCounter/resetCounter/counterName/counts methods
src/dsf/mobility/Street.cpp	Implements counter enabling, resetting, and incrementing on enqueue
src/dsf/mobility/Sensors.hpp	Simplifies Counter class to single count value with basic increment/reset
src/dsf/mobility/Sensors.cpp	File removed - Counter is now header-only
src/dsf/mobility/RoadNetwork.hpp	Renames makeSpireStreet to addCoil
src/dsf/mobility/RoadNetwork.cpp	Updates addCoil implementation, changes GeoJSON coilcode from uint64 to string
src/dsf/mobility/RoadDynamics.hpp	Removes meanSpireInputFlow/meanSpireOutputFlow, replaces saveInputStreetCounts/saveOutputStreetCounts with saveCoilCounts
src/dsf/bindings.cpp	Updates Python bindings to reflect renamed methods
test/mobility/Test_street.cpp	Replaces comprehensive SpireStreet tests with basic counter functionality test
test/mobility/Test_graph.cpp	Updates test to use addCoil instead of makeSpireStreet
test/mobility/Test_dynamics.cpp	Removes meanSpireFlow tests, updates to use hasCoil/addCoil
examples/stalingrado.cpp	Updates to use addCoil and counts() instead of SpireStreet
examples/slow_charge_tl.cpp	Replaces manual spire tracking with saveCoilCounts, removes PRINT_OUT_SPIRES
examples/slow_charge_rb.cpp	Replaces manual spire tracking with saveCoilCounts, removes PRINT_OUT_SPIRES
benchmark/Bench_Street.cpp	Renames SpireStreet benchmarks to CoilStreet, updates implementation
benchmark/Bench_Network.cpp	Code formatting improvements
benchmark/Bench_Dynamics.cpp	Code formatting improvements

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

src/dsf/mobility/Street.hpp

Copilot · 2025-11-07T09:48:00Z

examples/stalingrado.cpp

      }
+      coil->resetCounter();


Logic issue: The counter is reset every time progress % 60 == 0 (line 105), but counts are only written when progress % 300 == 0 (line 103). This means the counter gets reset at progress values 60, 120, 180, and 240 without saving those counts, causing data loss. Consider moving the reset inside the if (progress % 300 == 0) block or adjusting the logic to preserve all counts.

Suggested change

}

coil->resetCounter();

coil->resetCounter();

}

src/dsf/mobility/RoadNetwork.cpp

Copilot · 2025-11-07T09:48:00Z

src/dsf/mobility/RoadDynamics.hpp

    /// @brief Save the street input counts in csv format
    /// @param filename The name of the file
    /// @param reset If true, the input counts are cleared after the computation


Documentation inconsistency: The documentation says "Save the street input counts" but the method name is saveCoilCounts. Consider updating the documentation to match the method name, e.g., "Save the coil counts from streets with coils".

Suggested change

/// @brief Save the street input counts in csv format

/// @param filename The name of the file

/// @param reset If true, the input counts are cleared after the computation

/// @brief Save the coil counts from streets with coils in csv format

/// @param filename The name of the file

/// @param reset If true, the coil counts are cleared after the computation

src/dsf/mobility/Street.hpp

src/dsf/mobility/Street.cpp

Refactor coils

ba6f4b8

github-advanced-security bot found potential problems Nov 6, 2025

View reviewed changes

Fixing

74b6bbe

github-advanced-security bot found potential problems Nov 7, 2025

View reviewed changes

Grufoony requested a review from Copilot November 7, 2025 09:42

Copilot AI reviewed Nov 7, 2025

View reviewed changes

Grufoony added 4 commits November 7, 2025 11:13

A number is not a string, apparently

474d4f2

Copilot suggestions

fdc50f7

I don't think this is actually useful

62f89f1

Bump version

3c03a8d

Grufoony merged commit a09f7ed into main Nov 7, 2025
43 checks passed

Grufoony deleted the refactorCoils branch November 7, 2025 14:00

Refactor coils #364

Refactor coils #364

Uh oh!

Conversation

Grufoony commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Check warning

Check warning

Check warning

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

github-advanced-security bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Grufoony commented Nov 6, 2025 •

edited

Loading

codecov bot commented Nov 6, 2025 •

edited

Loading