From 36c7cf045f526e836adccec097444aecd0f79beb Mon Sep 17 00:00:00 2001
From: Romain BERNARD <romain.bernard@uca.fr>
Date: Mon, 24 Jun 2024 14:30:13 +0200
Subject: [PATCH] refactors for clarity (and fix heuristic), O/D node index
 were unnecessary in arguments as we have the KPs in

---
 src/routes/vehicle/SAEVRoute.cpp         | 70 +++++++++++++-----------
 src/routes/vehicle/SAEVRoute.h           | 16 ++++--
 test/src/BestInsertionHeuristicDebug.cpp | 12 ++--
 test/src/ConstraintPropagationDebug.cpp  | 12 ++--
 4 files changed, 63 insertions(+), 47 deletions(-)

diff --git a/src/routes/vehicle/SAEVRoute.cpp b/src/routes/vehicle/SAEVRoute.cpp
index 0f194d7..cbc1065 100644
--- a/src/routes/vehicle/SAEVRoute.cpp
+++ b/src/routes/vehicle/SAEVRoute.cpp
@@ -23,8 +23,8 @@ SAEVRoute::SAEVRoute(const Graph& graph, const std::vector<Request>& requestList
         getOrigin(i) = SAEVKeyPoint(graph, requestList.at(i), true); //origin
         getDestination(i) = SAEVKeyPoint(graph, requestList.at(i), false); //destination
         //Link Origins and Destinations
-        getOrigin(i).setCounterpart(&getDestination(i));
-        getDestination(i).setCounterpart(&getOrigin(i));
+        getRequestOrigin(i).setCounterpart(&getRequestDestination(i));
+        getRequestDestination(i).setCounterpart(&getRequestOrigin(i));
 
         //Create depot O/D KP (Upper Bound = nb requests)
         getOriginDepot(i) = SAEVKeyPoint(graph.getDepotNodeIdx()); //start
@@ -38,8 +38,8 @@ SAEVRoute::SAEVRoute(const Graph& graph, const std::vector<Request>& requestList
 }
 
 void SAEVRoute::insertRequest(size_t requestId, SAEVKeyPoint &originRequestPredecessorKP, SAEVKeyPoint &destinationRequestPredecessorKP) {
-    SAEVKeyPoint& originKp = getOrigin(requestId);
-    SAEVKeyPoint& destinationKp = getDestination(requestId);
+    SAEVKeyPoint& originKp = getRequestOrigin(requestId);
+    SAEVKeyPoint& destinationKp = getRequestDestination(requestId);
 
     SAEVKeyPoint *originSuccKp = originRequestPredecessorKP.getSuccessor();
     SAEVKeyPoint *destinationSuccKp = destinationRequestPredecessorKP.getSuccessor();
@@ -75,18 +75,25 @@ void SAEVRoute::removeRequest(size_t requestId) {
     //Before undoing the insertion, update weights on the route
     removeRequestWeightFromRoute(requestId);
 
-    SAEVKeyPoint& originKp = getOrigin(requestId);
-    SAEVKeyPoint& destinationKp = getDestination(requestId);
+    SAEVKeyPoint& originKp = getRequestOrigin(requestId);
+    SAEVKeyPoint& destinationKp = getRequestDestination(requestId);
 
     //get predecessor and successor for request
     SAEVKeyPoint* originPredecessor = originKp.getPredecessor();
-    SAEVKeyPoint* originSuccessor = destinationKp.getSuccessor();
-    SAEVKeyPoint* destinationPredecessor = originKp.getPredecessor();
+    SAEVKeyPoint* originSuccessor = originKp.getSuccessor();
+    SAEVKeyPoint* destinationPredecessor = destinationKp.getPredecessor();
     SAEVKeyPoint* destinationSuccessor = destinationKp.getSuccessor();
 
-    //Link pred and successor from origin and destination
-    originPredecessor->setSuccessor(originSuccessor);
-    destinationPredecessor->setSuccessor(destinationSuccessor);
+    //Link pred and successor from origin and destination (cases differ if O/D are next to each other
+    if(originSuccessor == &destinationKp) {
+        originPredecessor->setSuccessor(destinationSuccessor);
+        destinationSuccessor->setPredecessor(originPredecessor);
+    } else {
+        originPredecessor->setSuccessor(originSuccessor);
+        originSuccessor->setPredecessor(originPredecessor);
+        destinationPredecessor->setSuccessor(destinationSuccessor);
+        destinationSuccessor->setSuccessor(destinationPredecessor);
+    }
 
     //Revert origin/destination key points to their default state
     originKp.setPredecessor(nullptr);
@@ -115,7 +122,7 @@ SAEVRoute::tryAddRequest(const size_t requestId, SAEVKeyPoint &originRequestPred
     } while (currentKP != destinationSuccessor && currentKP != nullptr);
 
     //Do basic checks on neighbouring nodes from our Origin/Destination insertion points
-    bool isValid = doNeighbouringTWChecks(requestId, request->getOriginNodeIndex(), request->getDestinationNodeIndex(), &originRequestPredecessorKP, &destinationRequestPredecessorKP);
+    bool isValid = doNeighbouringTWChecks(requestId, &originRequestPredecessorKP, &destinationRequestPredecessorKP);
 
     if(isValid) {
         return insertRequestWithPropagation(requestId, originRequestPredecessorKP, destinationRequestPredecessorKP);
@@ -126,12 +133,13 @@ SAEVRoute::tryAddRequest(const size_t requestId, SAEVKeyPoint &originRequestPred
 }
 
 bool
-SAEVRoute::doNeighbouringTWChecks(const size_t requestId, const size_t originNodeIndex, const size_t destinationNodeIndex,
-                                  const SAEVKeyPoint *originPredecessor, const SAEVKeyPoint *destinationPredecessor) {
+SAEVRoute::doNeighbouringTWChecks(const size_t requestId, const SAEVKeyPoint *originPredecessor, const SAEVKeyPoint *destinationPredecessor) {
 
-    const SAEVKeyPoint& originKP = getOrigin(requestId);
-    const SAEVKeyPoint& destinationKP = getDestination(requestId);
+    const SAEVKeyPoint& originKP = getRequestOrigin(requestId);
+    const SAEVKeyPoint& destinationKP = getRequestDestination(requestId);
     const SAEVKeyPoint* originSuccessor = originPredecessor->getSuccessor();
+    const size_t originNodeIndex = originKP.getNodeIndex();
+    const size_t destinationNodeIndex = destinationKP.getNodeIndex();
 
     if(originPredecessor != destinationPredecessor)
     {
@@ -181,8 +189,8 @@ SAEVRouteChangelist SAEVRoute::insertRequestWithPropagation(const size_t request
 
     //Initialize bound propagation signal queue (each item signals a modification done on one of a KeyPoint
     std::queue<std::pair<Bound, SAEVKeyPoint *>> boundPropagationQueue{};
-    SAEVKeyPoint * originKP = &getOrigin(requestId);
-    SAEVKeyPoint * destinationKP = &getDestination(requestId);
+    SAEVKeyPoint * originKP = &getRequestOrigin(requestId);
+    SAEVKeyPoint * destinationKP = &getRequestDestination(requestId);
     boundPropagationQueue.emplace(Min, originKP->getPredecessor());
     boundPropagationQueue.emplace(Max, originKP->getSuccessor());
     boundPropagationQueue.emplace(Min, destinationKP->getPredecessor());
@@ -204,7 +212,7 @@ SAEVRouteChangelist SAEVRoute::insertRequestWithPropagation(const size_t request
         auto const& [bound, keyPoint] = boundPropagationQueue.front();
         boundPropagationQueue.pop();
         counterpartKP = keyPoint->getCounterpart();
-        DEBUG_MSG("KP=" + keyPoint->to_string() + "\n");
+//        DEBUG_MSG("KP=" + keyPoint->to_string() + "\n");
         if(bound == Min) {
             successorKP = keyPoint->getSuccessor();
             if(successorKP != nullptr) {
@@ -217,7 +225,7 @@ SAEVRouteChangelist SAEVRoute::insertRequestWithPropagation(const size_t request
                         changelist.setStatus(SAEVRouteChangelist::InsertionStatus::FAILURE_MIN);
                         return changelist;
                     }
-                    DEBUG_MSG("\tMIN Successeur KP=" + successorKP->to_string() + "\n\tModif Min=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
+//                    DEBUG_MSG("\tMIN Successeur KP=" + successorKP->to_string() + "\n\tModif Min=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
                     changelist.emplace_back(*successorKP, Min, newValue - oldValue);
                     successorKP->setMinTw(newValue);
                     boundPropagationQueue.emplace(Min, successorKP);
@@ -234,7 +242,7 @@ SAEVRouteChangelist SAEVRoute::insertRequestWithPropagation(const size_t request
                     changelist.setStatus(SAEVRouteChangelist::InsertionStatus::FAILURE_DELTA_MIN);
                     return changelist;
                 }
-                DEBUG_MSG("\tMIN Counterpart KP=" + counterpartKP->to_string() + "\n\tModif Min=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
+//                DEBUG_MSG("\tMIN Counterpart KP=" + counterpartKP->to_string() + "\n\tModif Min=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
                 changelist.emplace_back(*counterpartKP, Min, newValue - oldValue);
                 counterpartKP->setMinTw(newValue);
                 boundPropagationQueue.emplace(Min, counterpartKP);
@@ -253,7 +261,7 @@ SAEVRouteChangelist SAEVRoute::insertRequestWithPropagation(const size_t request
                         changelist.setStatus(SAEVRouteChangelist::InsertionStatus::FAILURE_MAX);
                         return changelist;
                     }
-                    DEBUG_MSG("\tMAX Predecessor KP=" + predecessorKP->to_string() + "\n\tModif Max=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
+//                    DEBUG_MSG("\tMAX Predecessor KP=" + predecessorKP->to_string() + "\n\tModif Max=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
                     changelist.emplace_back(*predecessorKP, Max, newValue - oldValue);
                     predecessorKP->setMaxTw(newValue);
                     boundPropagationQueue.emplace(Max, predecessorKP);
@@ -273,11 +281,12 @@ SAEVRouteChangelist SAEVRoute::insertRequestWithPropagation(const size_t request
                 changelist.emplace_back(*counterpartKP, Max, oldValue - newValue);
                 counterpartKP->setMaxTw(newValue);
                 boundPropagationQueue.emplace(Max, counterpartKP);
-                DEBUG_MSG("\tMAX Destination KP=" + counterpartKP->to_string() + "\n\tModif Max=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
+//                DEBUG_MSG("\tMAX Destination KP=" + counterpartKP->to_string() + "\n\tModif Max=" + std::to_string(oldValue) + "->" + std::to_string(newValue));
             }
         }
     }
 
+    DEBUG_MSG("INSERTION SUCCESS");
     changelist.setStatus(SAEVRouteChangelist::InsertionStatus::SUCCESS);
     changelist.setScore(getDetourScore(requestId, originRequestPredecessorKP, destinationRequestPredecessorKP));
     return changelist;
@@ -286,8 +295,8 @@ SAEVRouteChangelist SAEVRoute::insertRequestWithPropagation(const size_t request
 double SAEVRoute::getDetourScore(const size_t requestId, const SAEVKeyPoint &originRequestPredecessorKP,
                                  const SAEVKeyPoint &destinationRequestPredecessorKP) {
     double score;
-    const SAEVKeyPoint& originKP = getOrigin(requestId);
-    const SAEVKeyPoint& destinationKP = getDestination(requestId);
+    const SAEVKeyPoint& originKP = getRequestOrigin(requestId);
+    const SAEVKeyPoint& destinationKP = getRequestDestination(requestId);
     const SAEVKeyPoint* originSuccKP = originRequestPredecessorKP.getSuccessor();
     const SAEVKeyPoint* destinationSuccKP = destinationRequestPredecessorKP.getSuccessor();
 
@@ -406,8 +415,7 @@ BestInsertionQueue SAEVRoute::getBestInsertionsQueue(size_t requestId, size_t ve
     //iterate over possible origin/destination pairs for the given vehicle
     while(originInsertionKeyPoint->getSuccessor() != nullptr) {
         while(destinationInsertionKeyPoint->getSuccessor() != nullptr) {
-            if(doNeighbouringTWChecks(requestId, getRequestOriginIdx(requestId), getRequestDestinationIdx(requestId),
-                                      originInsertionKeyPoint, destinationInsertionKeyPoint)) {
+            if(doNeighbouringTWChecks(requestId,originInsertionKeyPoint, destinationInsertionKeyPoint)) {
                 score = getDetourScore(requestId, *originInsertionKeyPoint, *destinationInsertionKeyPoint);
                 bestInsertionQueue.emplace(originInsertionKeyPoint, destinationInsertionKeyPoint, score);
             }
@@ -423,24 +431,24 @@ BestInsertionQueue SAEVRoute::getBestInsertionsQueue(size_t requestId, size_t ve
 }
 
 void SAEVRoute::addRequestWeightToRoute(size_t requestId) {
-    SAEVKeyPoint* currentKP = &getOrigin(requestId);
+    SAEVKeyPoint* currentKP = &getRequestOrigin(requestId);
     int requestWeight = currentKP->getRequest()->getWeight();
     currentKP->setCurrentOccupation(currentKP->getPredecessor()->getCurrentOccupation() + requestWeight); //O.Weight = Prec(O).Weight + R.Weight (request enters the vehicle=)
     do {
         currentKP = currentKP->getSuccessor();
         currentKP->setCurrentOccupation(currentKP->getCurrentOccupation() + requestWeight);
-    } while (currentKP != &getDestination(requestId));
+    } while (currentKP != &getRequestDestination(requestId));
     currentKP->setCurrentOccupation(currentKP->getPredecessor()->getCurrentOccupation() - requestWeight); //D.Weight = Prec(D).Weight - R.Weight (request leaves the vehicle)
 }
 
 void SAEVRoute::removeRequestWeightFromRoute(size_t requestId) {
-    SAEVKeyPoint* currentKP = &getOrigin(requestId);
+    SAEVKeyPoint* currentKP = &getRequestOrigin(requestId);
     int requestWeight = currentKP->getRequest()->getWeight();
     currentKP->setCurrentOccupation(0); //reset request weight on origin KP
     do {
         currentKP = currentKP->getSuccessor();
         currentKP->setCurrentOccupation(currentKP->getCurrentOccupation() - requestWeight);
-    } while (currentKP != &getDestination(requestId));
+    } while (currentKP != &getRequestDestination(requestId));
     currentKP->setCurrentOccupation(0); //reset request weight on destination KP
 }
 
diff --git a/src/routes/vehicle/SAEVRoute.h b/src/routes/vehicle/SAEVRoute.h
index 394ed0f..df44ddc 100644
--- a/src/routes/vehicle/SAEVRoute.h
+++ b/src/routes/vehicle/SAEVRoute.h
@@ -84,8 +84,8 @@ public:
      * @param destinationPredecessor The destination's expected predecessor, aka the point after which we wish to insert our destination
      * @return true iff all neighbouring time window conditions are valid at our insertion points, false otherwise
      */
-    bool doNeighbouringTWChecks(const size_t requestId, const size_t originNodeIndex, const size_t destinationNodeIndex,
-                                const SAEVKeyPoint *originPredecessor, const SAEVKeyPoint *destinationPredecessor);
+    bool doNeighbouringTWChecks(const size_t requestId, const SAEVKeyPoint *originPredecessor,
+                                const SAEVKeyPoint *destinationPredecessor);
 
     /**
      * Method called after having validated conditions not requiring request insertion. \n
@@ -118,11 +118,15 @@ public:
     SAEVKeyPoint& getOriginDepot(const size_t vehicleId) { return _route.at(_nbRequest*2 + vehicleId*2);}
     SAEVKeyPoint& getDestinationDepot(const size_t vehicleId) { return _route.at(_nbRequest*2 + vehicleId*2 + 1);}
 
-    [[nodiscard]] size_t getRequestOriginIdx(const size_t requestId) const { return requestId * 2;}
-    [[nodiscard]] size_t getRequestDestinationIdx(const size_t requestId) const { return requestId * 2 + 1;}
+    [[nodiscard]] size_t getRequestOriginRouteIdx(const size_t requestId) const { return requestId * 2;}
+    [[nodiscard]] size_t getRequestDestinationRouteIdx(const size_t requestId) const { return requestId * 2 + 1;}
+    [[nodiscard]] size_t getRequestOriginNodeIdx(const size_t requestId) const { return _route[getRequestOriginRouteIdx(requestId)].getNodeIndex();}
+    [[nodiscard]] size_t getRequestDestinationNodeIdx(const size_t requestId) const { return _route[getRequestDestinationRouteIdx(requestId)].getNodeIndex();}
 
-    [[nodiscard]] size_t getOriginDepotIdx(const size_t vehicleId) const { return _nbRequest*2 + vehicleId*2;}
-    [[nodiscard]] size_t getDestinationDepotIdx(const size_t vehicleId) const { return _nbRequest*2 + vehicleId*2 + 1;}
+    [[nodiscard]] size_t getOriginDepotRouteIdx(const size_t vehicleId) const { return _nbRequest*2 + vehicleId*2;}
+    [[nodiscard]] size_t getDestinationDepotRouteIdx(const size_t vehicleId) const { return _nbRequest*2 + vehicleId*2 + 1;}
+    [[nodiscard]] size_t getOriginDepotNodeIdx(const size_t vehicleId) const { return _route[_nbRequest*2 + vehicleId*2].getNodeIndex();}
+    [[nodiscard]] size_t getDestinationDepotNodeIdx(const size_t vehicleId) const { return _route[_nbRequest*2 + vehicleId*2].getNodeIndex();}
 
     [[nodiscard]] size_t getLastActiveVehicleId() const { return _lastActiveVehicleId; }
     void setLastActiveVehicleId(size_t lastActiveVehicleId) { _lastActiveVehicleId = lastActiveVehicleId; }
diff --git a/test/src/BestInsertionHeuristicDebug.cpp b/test/src/BestInsertionHeuristicDebug.cpp
index 3d7c600..1649fb5 100644
--- a/test/src/BestInsertionHeuristicDebug.cpp
+++ b/test/src/BestInsertionHeuristicDebug.cpp
@@ -31,7 +31,8 @@ TEST(BestInsertionHeuristicDebug, DebugBaseInstance) {
     std::cout << routesContainer.to_string(vehicleId) << std::endl;
     assert(routesContainer.checkRouteTimeWindows(vehicleId));
     std::cout << "------------------------------------------------------------------" << std::endl;
-    SAEVRouteChangelist req1Changelist = routesContainer.tryAddRequest(1, routesContainer.getOriginDepot(vehicleId), routesContainer.getDestination(0));
+    SAEVRouteChangelist req1Changelist = routesContainer.tryAddRequest(1, routesContainer.getOriginDepot(vehicleId),
+                                                                       routesContainer.getRequestDestination(0));
     std::cout << routesContainer.to_string(vehicleId) << std::endl << std::endl;
     assert(!routesContainer.checkRouteTimeWindows(vehicleId));
     std::cout << "------------------------------------------------------------------" << std::endl;
@@ -61,18 +62,19 @@ TEST(BestInsertionQueueDebug, DebugInstanceAlain) {
     BestInsertionQueue biQueue = routesContainer.getBestInsertionsQueue(0,0);
     routesContainer.tryAddRequest(0,routesContainer.getOriginDepot(0),routesContainer.getOriginDepot(0));
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(1,0);
-    routesContainer.tryAddRequest(1,routesContainer.getOrigin(0),routesContainer.getOrigin(0));
+    routesContainer.tryAddRequest(1, routesContainer.getRequestOrigin(0), routesContainer.getRequestOrigin(0));
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(2,0);
-    SAEVRouteChangelist cl = routesContainer.tryAddRequest(2,routesContainer.getOrigin(1),routesContainer.getDestination(1));
+    SAEVRouteChangelist cl = routesContainer.tryAddRequest(2, routesContainer.getRequestOrigin(1),
+                                                           routesContainer.getRequestDestination(1));
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(3,0);
 
     //Vehicle 2 insertions
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(5,1);
     routesContainer.tryAddRequest(5,routesContainer.getOriginDepot(1),routesContainer.getOriginDepot(1));
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(4,1);
-    routesContainer.tryAddRequest(4,routesContainer.getOriginDepot(1),routesContainer.getDestination(5));
+    routesContainer.tryAddRequest(4,routesContainer.getOriginDepot(1), routesContainer.getRequestDestination(5));
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(3,1);
-    routesContainer.tryAddRequest(3,routesContainer.getOriginDepot(1),routesContainer.getOrigin(4));
+    routesContainer.tryAddRequest(3,routesContainer.getOriginDepot(1), routesContainer.getRequestOrigin(4));
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(0,1);
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(1,1);
     biQueue = routesContainer.getBestFeasibleInsertionsQueue(2,1);
diff --git a/test/src/ConstraintPropagationDebug.cpp b/test/src/ConstraintPropagationDebug.cpp
index dd68aad..5dd841d 100644
--- a/test/src/ConstraintPropagationDebug.cpp
+++ b/test/src/ConstraintPropagationDebug.cpp
@@ -31,7 +31,8 @@ TEST(ConstraintPropagationDebug, DebugBaseInstance) {
     std::cout << routesContainer.to_string(vehicleId) << std::endl;
     assert(routesContainer.checkRouteTimeWindows(vehicleId));
     std::cout << "------------------------------------------------------------------" << std::endl;
-    SAEVRouteChangelist req1Changelist = routesContainer.tryAddRequest(1, routesContainer.getOriginDepot(vehicleId), routesContainer.getDestination(0));
+    SAEVRouteChangelist req1Changelist = routesContainer.tryAddRequest(1, routesContainer.getOriginDepot(vehicleId),
+                                                                       routesContainer.getRequestDestination(0));
     std::cout << routesContainer.to_string(vehicleId) << std::endl << std::endl;
     assert(!routesContainer.checkRouteTimeWindows(vehicleId));
     std::cout << "------------------------------------------------------------------" << std::endl;
@@ -80,13 +81,14 @@ TEST(ConstraintPropagationDebug, DebugInstanceAlain) {
 
     //Vehicle 1 insertions
     routesContainer.tryAddRequest(0,routesContainer.getOriginDepot(0),routesContainer.getOriginDepot(0));
-    routesContainer.tryAddRequest(1,routesContainer.getOrigin(0),routesContainer.getOrigin(0));
-    SAEVRouteChangelist cl = routesContainer.tryAddRequest(2,routesContainer.getOrigin(1),routesContainer.getDestination(1));
+    routesContainer.tryAddRequest(1, routesContainer.getRequestOrigin(0), routesContainer.getRequestOrigin(0));
+    SAEVRouteChangelist cl = routesContainer.tryAddRequest(2, routesContainer.getRequestOrigin(1),
+                                                           routesContainer.getRequestDestination(1));
 
     //Vehicle 2 insertions
     routesContainer.tryAddRequest(5,routesContainer.getOriginDepot(1),routesContainer.getOriginDepot(1));
-    routesContainer.tryAddRequest(4,routesContainer.getOriginDepot(1),routesContainer.getDestination(5));
-    routesContainer.tryAddRequest(3,routesContainer.getOriginDepot(1),routesContainer.getOrigin(4));
+    routesContainer.tryAddRequest(4,routesContainer.getOriginDepot(1), routesContainer.getRequestDestination(5));
+    routesContainer.tryAddRequest(3,routesContainer.getOriginDepot(1), routesContainer.getRequestOrigin(4));
 }
 
 }
-- 
GitLab