Skip to content

Commit

Permalink
Move the data from the nodesTentativeTime to a mutable inside the nod…
Browse files Browse the repository at this point in the history 
…e object

Fetching the temporary data from a map in the calculation loop is too intensive.
We this, we move the data inside the node object. Since the basic Node
object is const, we add the variable as mutable. We can use this concept
for more temporary data object.
This concept only work as long as we are single threaded.
  • Loading branch information
@greenscientist
greenscientist committed Oct 20, 2023
1 parent cf1956f commit ab53145
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 40 deletions.
1 change: 0 additions & 1 deletion connection_scan_algorithm/include/calculator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ namespace TrRouting
int minEgressTravelTime;
long long calculationTime;

std::unordered_map<Node::uid_t, int> nodesTentativeTime; // arrival time at node
std::unordered_map<Node::uid_t, int> nodesReverseTentativeTime; // departure time at node
std::unordered_map<Node::uid_t, NodeTimeDistance> nodesAccess; // travel time/distance from origin to accessible nodes
std::unordered_map<Node::uid_t, NodeTimeDistance> nodesEgress; // travel time/distance to reach destination;
Expand Down
44 changes: 9 additions & 35 deletions connection_scan_algorithm/src/forward_calculation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,8 @@ namespace TrRouting
std::optional<std::reference_wrapper<const Connection>> tripEnterConnection = currentTripQueryOverlay.enterConnection;
const Node &nodeDeparture = (*connection).get().getDepartureNode();

// Extract node departure time if we have a result or set a default value
auto ite = nodesTentativeTime.find(nodeDeparture.uid);
if (ite != nodesTentativeTime.end()) {
nodeDepartureTentativeTime = ite->second;
} else{
nodeDepartureTentativeTime = MAX_INT;
}
// Extract node departure time that was set or use its default value
nodeDepartureTentativeTime = nodeDeparture.tentativeTime;

// TODO Do we need to make sure the departure node exists in the forwardJourneySteps map? For the reverse calculation, we had to in order to fix issue https://github.com/chairemobilite/trRouting/issues/250 The issue may apply to forward too, but we have no example
auto nodesAccessIte = nodesAccess.find(nodeDeparture.uid);
Expand Down Expand Up @@ -123,13 +118,7 @@ namespace TrRouting
for (const NodeTimeDistance & transferableNode : nodeArrival.transferableNodes)
{
// Extract tentative time for current transferable node if found
int currentTransferablenNodesTentativeTime = 0;
auto nodesIte = nodesTentativeTime.find(transferableNode.node.uid);
if (nodesIte != nodesTentativeTime.end()) {
currentTransferablenNodesTentativeTime = nodesIte->second;
} else {
currentTransferablenNodesTentativeTime = MAX_INT;
}
int currentTransferablenNodesTentativeTime = transferableNode.node.tentativeTime;

if (nodeArrival != transferableNode.node &&
currentTransferablenNodesTentativeTime < connectionArrivalTime)
Expand All @@ -145,8 +134,7 @@ namespace TrRouting
if (footpathTravelTime + connectionArrivalTime < currentTransferablenNodesTentativeTime)
{
footpathDistance = nodeArrival.transferableNodes[footpathIndex].distance;
nodesTentativeTime[transferableNode.node.uid] = footpathTravelTime + connectionArrivalTime;

transferableNode.node.tentativeTime = footpathTravelTime + connectionArrivalTime;
//TODO DO we need a make_optional here??
forwardJourneysSteps.insert_or_assign(transferableNode.node.uid, JourneyStep(currentTripQueryOverlay.enterConnection, *connection, std::cref(trip), footpathTravelTime, (nodeArrival == transferableNode.node), footpathDistance));
}
Expand Down Expand Up @@ -266,14 +254,8 @@ namespace TrRouting
std::optional<std::reference_wrapper<const Connection>> tripEnterConnection = currentTripQueryOverlay.enterConnection;
const Node &nodeDeparture = (*connection).get().getDepartureNode();

// Extract node departure time if we have a result or set a default value
auto ite = nodesTentativeTime.find(nodeDeparture.uid);
if (ite != nodesTentativeTime.end()) {
nodeDepartureTentativeTime = ite->second;
} else{
nodeDepartureTentativeTime = MAX_INT;
}

// Extract node departure time if it was set or use the default value
nodeDepartureTentativeTime = nodeDeparture.tentativeTime;
auto nodesAccessIte = nodesAccess.find(nodeDeparture.uid);
nodeWasAccessedFromOrigin = parameters.getMaxFirstWaitingTimeSeconds() > 0 &&
nodesAccessIte != nodesAccess.end() &&
Expand Down Expand Up @@ -312,15 +294,8 @@ namespace TrRouting
footpathIndex = 0;
for (const NodeTimeDistance & transferableNode : nodeArrival.transferableNodes)
{
// Extract tentative time for current transferable node if found
int currentTransferablenNodesTentativeTime = 0;
auto nodesIte = nodesTentativeTime.find(transferableNode.node.uid);
if (nodesIte != nodesTentativeTime.end()) {
currentTransferablenNodesTentativeTime = nodesIte->second;
} else {
currentTransferablenNodesTentativeTime = MAX_INT;
}

// Extract tentative time for current transferable node. MAX_INT is default
int currentTransferablenNodesTentativeTime = transferableNode.node.tentativeTime;
if (nodeArrival != transferableNode.node &&
currentTransferablenNodesTentativeTime < connectionArrivalTime)
{
Expand All @@ -335,8 +310,7 @@ namespace TrRouting
if (footpathTravelTime + connectionArrivalTime < currentTransferablenNodesTentativeTime)
{
footpathDistance = nodeArrival.transferableNodes[footpathIndex].distance;
nodesTentativeTime[transferableNode.node.uid] = footpathTravelTime + connectionArrivalTime;

transferableNode.node.tentativeTime = footpathTravelTime + connectionArrivalTime;
//TODO DO we need a make_optional here??
forwardJourneysSteps.insert_or_assign(transferableNode.node.uid, JourneyStep(currentTripQueryOverlay.enterConnection, *connection, std::cref(trip), footpathTravelTime, (nodeArrival == transferableNode.node), footpathDistance));
}
Expand Down
1 change: 0 additions & 1 deletion connection_scan_algorithm/src/initializations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ namespace TrRouting
spdlog::info("preparing nodes tentative times, trips enter connections and journeys...");


nodesTentativeTime.clear();
nodesReverseTentativeTime.clear();
nodesAccess.clear();
nodesEgress.clear();
Expand Down
9 changes: 7 additions & 2 deletions connection_scan_algorithm/src/resets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ namespace TrRouting
minEgressTravelTime = MAX_INT;
maxAccessTravelTime = -1;

// Clear temporary calculation data
for (auto &&[uuid,node] : transitData.getNodes()) {
node.resetMutables();
}

//TODO Question, do we only use accessFootpath when those condtion are true? The whole calculation should probably
// be a different path in this case.
if (origin.has_value())
Expand All @@ -80,7 +85,7 @@ namespace TrRouting
int footpathDistanceMeters;
nodesAccess.clear();
forwardJourneysSteps.clear();
nodesTentativeTime.clear();

for (auto & accessFootpath : accessFootpaths)
{
footpathTravelTimeSeconds = (int)ceil((float)(accessFootpath.time) / params.walkingSpeedFactor);
Expand All @@ -90,7 +95,7 @@ namespace TrRouting
footpathTravelTimeSeconds,
footpathDistanceMeters));
forwardJourneysSteps.insert_or_assign(accessFootpath.node.uid, JourneyStep(std::nullopt, std::nullopt, std::nullopt, footpathTravelTimeSeconds, false, footpathDistanceMeters));
nodesTentativeTime[accessFootpath.node.uid] = departureTimeSeconds + footpathTravelTimeSeconds;
accessFootpath.node.tentativeTime = departureTimeSeconds + footpathTravelTimeSeconds;
if (footpathTravelTimeSeconds < minAccessTravelTime)
{
minAccessTravelTime = footpathTravelTimeSeconds;
Expand Down
11 changes: 10 additions & 1 deletion include/node.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <string>
#include <boost/uuid/uuid.hpp>
#include <boost/uuid/uuid_io.hpp>

#include <limits>
#include "point.hpp"
#include <boost/functional/hash.hpp>

Expand All @@ -32,6 +32,7 @@ namespace TrRouting
uid(++global_uid)
{
point = std::move(_point);
resetMutables();
}

boost::uuids::uuid uuid;
Expand All @@ -43,6 +44,14 @@ namespace TrRouting
std::unique_ptr<Point> point; //TODO Does this need to be a ptr or could be part of the object?
std::vector<NodeTimeDistance> transferableNodes;
std::vector<NodeTimeDistance> reverseTransferableNodes; //TODO Add comment on what this is
// These mutable components contain temporary data used during the calculation
// We'll need a different solution to implement multithreaded computation
mutable int tentativeTime;

// Return temporary data to their initial values
void resetMutables() const {
tentativeTime = std::numeric_limits<int>::max();
}

const std::string toString() {
return "Node " + boost::uuids::to_string(uuid) + " (id " + std::to_string(id) + ")\n code " + code + "\n name " + name + "\n latitude " + std::to_string(point.get()->latitude) + "\n longitude " + std::to_string(point.get()->longitude);
Expand Down

0 comments on commit ab53145

Please sign in to comment.