Skip to content

Commit

Permalink
Remove unnecessary footpathIndex from calculation
Browse files Browse the repository at this point in the history 
The iterator that was used in the loop was pointing to the same position
as the footpath index.

Benchmarks seems to show a small perf improvement in most cases

Tested in transition, results are not effected
  • Loading branch information
@greenscientist
greenscientist committed Dec 22, 2024
1 parent 8102f81 commit c9719b3
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 24 deletions.
20 changes: 6 additions & 14 deletions connection_scan_algorithm/src/forward_calculation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ namespace TrRouting
short connectionMinWaitingTimeSeconds {-1};
//long long footpathsRangeStart {-1};
//long long footpathsRangeEnd {-1};
int footpathIndex {-1};
int footpathTravelTime {-1};
int footpathDistance {-1};
int tentativeEgressNodeArrivalTime {MAX_INT};
Expand Down Expand Up @@ -110,7 +109,6 @@ namespace TrRouting
tentativeEgressNodeArrivalTime = connectionArrivalTime;
}

footpathIndex = 0;
for (const NodeTimeDistance & transferableNode : nodeArrival.transferableNodes)
{
// Extract tentative time for current transferable node if found
Expand All @@ -119,18 +117,17 @@ namespace TrRouting
if (nodeArrival != transferableNode.node &&
currentTransferablenNodesTentativeTime < connectionArrivalTime)
{
footpathIndex++;
continue;
}

//TODO We should not do a direct == with float values
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? nodeArrival.transferableNodes[footpathIndex].time : (int)ceil((float)nodeArrival.transferableNodes[footpathIndex].time / parameters.getWalkingSpeedFactor());
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? transferableNode.time : (int)ceil((float)transferableNode.time / parameters.getWalkingSpeedFactor());

if (footpathTravelTime <= parameters.getMaxTransferWalkingTravelTimeSeconds())
{
if (footpathTravelTime + connectionArrivalTime < currentTransferablenNodesTentativeTime)
{
footpathDistance = nodeArrival.transferableNodes[footpathIndex].distance;
footpathDistance = transferableNode.distance;
nodesTentativeTime[transferableNode.node.uid] = footpathTravelTime + connectionArrivalTime;

//TODO DO we need a make_optional here??
Expand All @@ -148,11 +145,10 @@ namespace TrRouting
)
)
{
footpathDistance = nodeArrival.transferableNodes[footpathIndex].distance;
footpathDistance = transferableNode.distance;
forwardEgressJourneysSteps.insert_or_assign(transferableNode.node.uid, JourneyStep(currentTripQueryOverlay.enterConnection, *connection, std::cref(trip), footpathTravelTime, true, footpathDistance));
}
}
footpathIndex++;
}
}
reachableConnectionsCount++;
Expand Down Expand Up @@ -212,7 +208,6 @@ namespace TrRouting
short connectionMinWaitingTimeSeconds {-1};
//long long footpathsRangeStart {-1};
//long long footpathsRangeEnd {-1};
int footpathIndex {-1};
int footpathTravelTime {-1};
int footpathDistance {-1};
bool nodeWasAccessedFromOrigin {false};
Expand Down Expand Up @@ -286,7 +281,6 @@ namespace TrRouting
const Node &nodeArrival = (*connection).get().getArrivalNode();
connectionArrivalTime = (*connection).get().getArrivalTime();

footpathIndex = 0;
for (const NodeTimeDistance & transferableNode : nodeArrival.transferableNodes)
{
// Extract tentative time for current transferable node if found
Expand All @@ -295,18 +289,17 @@ namespace TrRouting
if (nodeArrival != transferableNode.node &&
currentTransferablenNodesTentativeTime < connectionArrivalTime)
{
footpathIndex++;
continue;
}

//TODO We should not do a direct == with float values
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? nodeArrival.transferableNodes[footpathIndex].time : (int)ceil((float)nodeArrival.transferableNodes[footpathIndex].time / parameters.getWalkingSpeedFactor());
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? transferableNode.time : (int)ceil((float)transferableNode.time / parameters.getWalkingSpeedFactor());

if (footpathTravelTime <= parameters.getMaxTransferWalkingTravelTimeSeconds())
{
if (footpathTravelTime + connectionArrivalTime < currentTransferablenNodesTentativeTime)
{
footpathDistance = nodeArrival.transferableNodes[footpathIndex].distance;
footpathDistance = transferableNode.distance;
nodesTentativeTime[transferableNode.node.uid] = footpathTravelTime + connectionArrivalTime;

//TODO DO we need a make_optional here??
Expand All @@ -324,11 +317,10 @@ namespace TrRouting
)
)
{
footpathDistance = nodeArrival.transferableNodes[footpathIndex].distance;
footpathDistance = transferableNode.distance;
forwardEgressJourneysSteps.insert_or_assign(transferableNode.node.uid, JourneyStep(currentTripQueryOverlay.enterConnection, *connection, std::cref(trip), footpathTravelTime, true, footpathDistance));
}
}
footpathIndex++;
}
}
reachableConnectionsCount++;
Expand Down
15 changes: 5 additions & 10 deletions connection_scan_algorithm/src/reverse_calculation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ namespace TrRouting
tentativeAccessNodeDepartureTime = connectionDepartureTime;

}
footpathIndex = 0;

for (const NodeTimeDistance & transferableNode : nodeDeparture.reverseTransferableNodes)
{

Expand All @@ -131,13 +131,13 @@ namespace TrRouting
}

//TODO We should not do a direct == with float values
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? nodeDeparture.reverseTransferableNodes[footpathIndex].time : (int)ceil((float)nodeDeparture.reverseTransferableNodes[footpathIndex].time / parameters.getWalkingSpeedFactor());
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? transferableNode.time : (int)ceil((float)transferableNode.time / parameters.getWalkingSpeedFactor());

if (footpathTravelTime <= parameters.getMaxTransferWalkingTravelTimeSeconds())
{
if (connectionDepartureTime - footpathTravelTime - connectionMinWaitingTimeSeconds >= nodesReverseTentativeTime.at(transferableNode.node.uid))
{
footpathDistance = nodeDeparture.reverseTransferableNodes.at(footpathIndex).distance;
footpathDistance = transferableNode.distance;
nodesReverseTentativeTime[transferableNode.node.uid] = connectionDepartureTime - footpathTravelTime - connectionMinWaitingTimeSeconds;
//TODO Do we need a make_optional<...>(connection) ??
reverseJourneysSteps.at(transferableNode.node.uid) = JourneyStep(*connection, currentTripQueryOverlay.exitConnection, std::cref(trip), footpathTravelTime, (nodeDeparture == transferableNode.node), footpathDistance);
Expand Down Expand Up @@ -173,7 +173,6 @@ namespace TrRouting
}
}
}
footpathIndex++;
}
}
reachableConnectionsCount++;
Expand Down Expand Up @@ -232,7 +231,6 @@ namespace TrRouting
short journeyConnectionMinWaitingTimeSeconds {-1};
//long long footpathsRangeStart {-1};
//long long footpathsRangeEnd {-1};
int footpathIndex {-1};
int footpathTravelTime {-1};
int footpathDistance {-1};

Expand Down Expand Up @@ -317,24 +315,22 @@ namespace TrRouting
connectionMinWaitingTimeSeconds = (*connection).get().getMinWaitingTimeOrDefault(parameters.getMinWaitingTimeSeconds());

auto nodeDepartureInNodesAccessIte = nodesAccess.find(nodeDeparture.uid);
footpathIndex = 0;
for (const NodeTimeDistance & transferableNode : nodeDeparture.reverseTransferableNodes)
{

if (nodeDeparture != transferableNode.node && nodesReverseTentativeTime.at(transferableNode.node.uid) > connectionDepartureTime - connectionMinWaitingTimeSeconds)
{
footpathIndex++;
continue;
}

//TODO We should not do a direct == with float values
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? nodeDeparture.reverseTransferableNodes[footpathIndex].time : (int)ceil((float)nodeDeparture.reverseTransferableNodes[footpathIndex].time / parameters.getWalkingSpeedFactor());
footpathTravelTime = parameters.getWalkingSpeedFactor() == 1.0 ? transferableNode.time : (int)ceil((float)transferableNode.time / parameters.getWalkingSpeedFactor());

if (footpathTravelTime <= parameters.getMaxTransferWalkingTravelTimeSeconds())
{
if (connectionDepartureTime - footpathTravelTime - connectionMinWaitingTimeSeconds >= nodesReverseTentativeTime.at(transferableNode.node.uid))
{
footpathDistance = nodeDeparture.reverseTransferableNodes.at(footpathIndex).distance;
footpathDistance = transferableNode.distance;
nodesReverseTentativeTime[transferableNode.node.uid] = connectionDepartureTime - footpathTravelTime - connectionMinWaitingTimeSeconds;
//TODO Do we need a make_optional<...>(connection) ??
reverseJourneysSteps.at(transferableNode.node.uid) = JourneyStep(*connection, currentTripQueryOverlay.exitConnection, std::cref(trip), footpathTravelTime, (nodeDeparture == transferableNode.node), footpathDistance);
Expand Down Expand Up @@ -370,7 +366,6 @@ namespace TrRouting
}
}
}
footpathIndex++;
}
}
reachableConnectionsCount++;
Expand Down

0 comments on commit c9719b3

Please sign in to comment.