Refactor `Network` class #315

Grufoony · 2025-09-08T16:03:24Z

Close Make Network auto manage ids #274
Close Replace Graph class with boost::graph #253 (not worth it)
Close [PROPOSAL] Add force parameter to Graph add... functions #208 (not useful feature)

codecov · 2025-09-08T16:06:34Z

Codecov Report

❌ Patch coverage is 74.20237% with 283 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.91%. Comparing base (18482cc) to head (439914d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/dsf/headers/RoadDynamics.hpp	58.76%	127 Missing ⚠️
src/dsf/sources/RoadNetwork.cpp	68.82%	53 Missing ⚠️
src/dsf/sources/Itinerary.cpp	11.11%	32 Missing ⚠️
src/dsf/sources/AdjacencyMatrix.cpp	17.39%	19 Missing ⚠️
src/dsf/headers/Street.hpp	0.00%	11 Missing ⚠️
src/dsf/headers/RoadJunction.hpp	0.00%	10 Missing ⚠️
src/dsf/utility/Logger.hpp	28.57%	10 Missing ⚠️
src/dsf/headers/Agent.hpp	0.00%	9 Missing ⚠️
src/dsf/headers/Network.hpp	89.09%	6 Missing ⚠️
src/dsf/sources/Street.cpp	57.14%	3 Missing ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
- Coverage   84.25%   80.91%   -3.35%     
==========================================
  Files          39       40       +1     
  Lines        5709     5448     -261     
  Branches      574      539      -35     
==========================================
- Hits         4810     4408     -402     
- Misses        899     1040     +141

Flag	Coverage Δ
unittests	`80.91% <74.20%> (-3.35%)`	⬇️

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.

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

Copilot reviewed 39 out of 40 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-12T16:25:26Z

src/dsf/sources/Street.cpp

+          strLaneMapping +=
+              std::format("{} - ", directionToString[static_cast<size_t>(item)]);


Array access with static_cast could be unsafe if item is out of bounds. The directionToString array has 7 elements (indices 0-6), but there's no bounds checking before the cast and array access.

Copilot · 2025-09-12T16:25:27Z

src/dsf/headers/Network.hpp

+  Id Network<node_t, edge_t>::m_cantorHash(Id u, Id v) const {
+    return ((u + v) * (u + v + 1)) / 2 + v;


The Cantor pairing function could cause integer overflow for large node IDs. Consider adding overflow checks or using a larger integer type for intermediate calculations.

Copilot · 2025-09-12T16:25:27Z

src/dsf/headers/RoadNetwork.hpp

    ///           The first input number is the number of nodes, followed by the coordinates of each node.
    ///           In the i-th row of the file, the (i - 1)-th node's coordinates are expected.
-    void importCoordinates(const std::string& fileName);
+    [[deprecated]] void importCoordinates(const std::string& fileName);


The deprecation attribute lacks a reason or replacement suggestion. Consider adding a deprecation message like [[deprecated(\"Use addNode with coordinates instead\")]] to help users migrate.

Grufoony · 2025-09-12T16:28:13Z

However, this runs 3x slower wrt main...

Grufoony · 2025-09-13T11:08:06Z

Ok I have finally found the impostor...

I think we can merge it, @sbaldu?

Copilot

Pull Request Overview

Copilot reviewed 40 out of 41 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-13T11:09:24Z

utils/get_osm_data.py

            gdf_edges = gdf_edges[
                ["osmid", "u", "v", "length", "lanes", "highway", "maxspeed", "name"]
            ]
+    gdf_edges["osmid"] = range(1, len(gdf_edges) + 1)


[nitpick] The edge ID reassignment uses 1-based indexing which may cause confusion since most programming contexts use 0-based indexing. Consider using range(len(gdf_edges)) for consistency with typical array indexing patterns.

Suggested change

gdf_edges["osmid"] = range(1, len(gdf_edges) + 1)

gdf_edges["osmid"] = range(len(gdf_edges))

Copilot · 2025-09-13T11:09:25Z

test/Test_street.cpp

    CHECK_EQ(street.angle(), 0);
    street.setGeometry(std::vector<std::pair<double, double>>{{1, 0}, {0, 1}});
-    CHECK_EQ(street.angle(), 3 * std::numbers::pi / 4);
+    CHECK_EQ(street.angle(), 7 * std::numbers::pi / 4);


The angle calculation appears incorrect. An angle of 7π/4 radians (315°) represents a direction that goes from (1,0) to (0,1), but this should be 3π/4 radians (135°) based on the coordinate change. The current value suggests a coordinate system interpretation issue.

Suggested change

CHECK_EQ(street.angle(), 7 * std::numbers::pi / 4);

CHECK_EQ(street.angle(), 3 * std::numbers::pi / 4);

Copilot · 2025-09-13T11:09:25Z

src/dsf/headers/RoadNetwork.hpp

    ///           The first input number is the number of nodes, followed by the coordinates of each node.
    ///           In the i-th row of the file, the (i - 1)-th node's coordinates are expected.
-    void importCoordinates(const std::string& fileName);
+    [[deprecated]] void importCoordinates(const std::string& fileName);


[nitpick] Multiple methods are marked as deprecated without providing information about their replacements or migration path. Consider adding deprecation messages that guide users to the new APIs, e.g., [[deprecated(\"Use newMethod() instead\")]].

Suggested change

[[deprecated]] void importCoordinates(const std::string& fileName);

[[deprecated("Use importCoordinates(const std::string&) with CSV-like files instead")]] void importCoordinates(const std::string& fileName);

Copilot · 2025-09-13T11:09:25Z

src/dsf/headers/Network.hpp

+  Id Network<node_t, edge_t>::m_cantorHash(Id u, Id v) const {
+    return ((u + v) * (u + v + 1)) / 2 + v;
  }


The Cantor pairing function can overflow for large values of u and v. Consider adding overflow checks or using a 64-bit integer type to prevent undefined behavior when the sum becomes large.

src/dsf/sources/Itinerary.cpp

+          std::format("Could not open file: {}", fileName)));
+    }
+    // Load the m_path variable from the file
+    inFile.read(reinterpret_cast<char*>(&m_destination), sizeof(Id));


src/dsf/sources/Itinerary.cpp

+    // Load the m_path variable from the file
+    inFile.read(reinterpret_cast<char*>(&m_destination), sizeof(Id));
+    size_t mapSize;
+    inFile.read(reinterpret_cast<char*>(&mapSize), sizeof(size_t));


src/dsf/sources/Itinerary.cpp

+    m_path.reserve(mapSize);
+    for (size_t i = 0; i < mapSize; ++i) {
+      Id key;
+      inFile.read(reinterpret_cast<char*>(&key), sizeof(Id));


src/dsf/sources/Itinerary.cpp

+      Id key;
+      inFile.read(reinterpret_cast<char*>(&key), sizeof(Id));
+      size_t vecSize;
+      inFile.read(reinterpret_cast<char*>(&vecSize), sizeof(size_t));


src/dsf/sources/Itinerary.cpp

+      size_t vecSize;
+      inFile.read(reinterpret_cast<char*>(&vecSize), sizeof(size_t));
+      std::vector<Id> vec(vecSize);
+      inFile.read(reinterpret_cast<char*>(vec.data()), vecSize * sizeof(Id));


Grufoony added 2 commits September 8, 2025 18:01

Increment cpp version to 23

edf5012

Init refactoring Network class

6c405a1

Grufoony added test Regarding the tests of the code improve Improving an already existing feature dynamics Regarding the dynamics of the system performance Regarding the performance of the code priority::high High priority stuff code refactoring examples Example simulations labels Sep 8, 2025

Grufoony self-assigned this Sep 8, 2025

github-advanced-security bot found potential problems Sep 8, 2025

View reviewed changes

Grufoony and others added 6 commits September 8, 2025 23:53

Some other tests

cfa5f2b

Draft

e656736

Revert to c++20 and refactoring

3b2d8d4

Fixing tests

01e9b4f

My working day has gone

c631ce4

Enhance formatters

739bb25

Grufoony force-pushed the refactorNetworkClass branch 2 times, most recently from 334db68 to 2d6f0aa Compare September 10, 2025 10:47

Update code and tests

6347dce

Grufoony force-pushed the refactorNetworkClass branch from 2d6f0aa to 6347dce Compare September 10, 2025 10:50

Grufoony added 6 commits September 10, 2025 15:45

Adjusting functions...

b4c43d4

Rework examples

a3fd633

Fix mac tests

7c4bb63

Bugfix

f10adbd

Merge branch 'main' into refactorNetworkClass

87d3291

Fix benchmarks

466fdf9

Grufoony marked this pull request as ready for review September 11, 2025 12:09

Grufoony requested a review from Copilot September 11, 2025 12:09

Grufoony added 2 commits September 11, 2025 16:00

Optimization

d39e65f

Fix binding

c9aeb47

Grufoony force-pushed the refactorNetworkClass branch 4 times, most recently from 0fd6b3c to dcebff0 Compare September 12, 2025 09:45

Prova velocità

44f8c78

Grufoony force-pushed the refactorNetworkClass branch from dcebff0 to 44f8c78 Compare September 12, 2025 13:05

Fixes

7e5c137

Grufoony force-pushed the refactorNetworkClass branch from bfd0360 to 7e5c137 Compare September 12, 2025 16:18

Grufoony requested a review from Copilot September 12, 2025 16:24

Copilot AI reviewed Sep 12, 2025

View reviewed changes

Optimization

6d25102

Grufoony force-pushed the refactorNetworkClass branch from b21716e to 6d25102 Compare September 12, 2025 17:39

Optimizations

db4d08e

Grufoony force-pushed the refactorNetworkClass branch from 297285e to db4d08e Compare September 12, 2025 19:01

Bugfixes

fc3b0e8

Grufoony requested a review from Copilot September 13, 2025 11:08

Copilot AI reviewed Sep 13, 2025

View reviewed changes

Optimization

2cc87c4

Grufoony mentioned this pull request Sep 13, 2025

Replace Graph class with boost::graph #253

Closed

Grufoony added 3 commits September 13, 2025 14:55

Try to fix benchmarks

6ef1cdd

Prova fix

1849cbf

Removing adjMatrix

439914d

Grufoony force-pushed the refactorNetworkClass branch from ee3f643 to 439914d Compare September 13, 2025 19:09

github-advanced-security bot found potential problems Sep 13, 2025

View reviewed changes

Grufoony merged commit dcbda37 into main Sep 15, 2025
29 of 31 checks passed

Grufoony deleted the refactorNetworkClass branch September 15, 2025 07:04

		strLaneMapping +=
		std::format("{} - ", directionToString[static_cast<size_t>(item)]);

		Id Network<node_t, edge_t>::m_cantorHash(Id u, Id v) const {
		return ((u + v) * (u + v + 1)) / 2 + v;

	gdf_edges["osmid"] = range(1, len(gdf_edges) + 1)
	gdf_edges["osmid"] = range(len(gdf_edges))

	CHECK_EQ(street.angle(), 7 * std::numbers::pi / 4);
	CHECK_EQ(street.angle(), 3 * std::numbers::pi / 4);

	[[deprecated]] void importCoordinates(const std::string& fileName);
	[[deprecated("Use importCoordinates(const std::string&) with CSV-like files instead")]] void importCoordinates(const std::string& fileName);

Refactor Network class #315

Refactor Network class #315

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

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

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Grufoony commented Sep 12, 2025

Uh oh!

Grufoony commented Sep 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Refactor `Network` class #315

Refactor `Network` class #315

Grufoony commented Sep 8, 2025 •

edited

Loading

codecov bot commented Sep 8, 2025 •

edited

Loading