From d77d1d5275971637056dc327495766c666288e4f Mon Sep 17 00:00:00 2001 From: lostsquirrel Date: Sun, 8 Dec 2024 20:06:52 +0000 Subject: [PATCH 1/7] stablize sorts --- rts/Sim/Path/HAPFS/PathDataTypes.h | 9 ++++++++- rts/Sim/Path/QTPFS/PathThreads.h | 24 +++++++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/rts/Sim/Path/HAPFS/PathDataTypes.h b/rts/Sim/Path/HAPFS/PathDataTypes.h index b108106aa7..909fb34c38 100644 --- a/rts/Sim/Path/HAPFS/PathDataTypes.h +++ b/rts/Sim/Path/HAPFS/PathDataTypes.h @@ -39,7 +39,14 @@ struct PathNode { /// functor to define node priority struct lessCost { inline bool operator() (const PathNode* x, const PathNode* y) const { - return (x->fCost == y->fCost) ? (x->gCost < y->gCost) : (x->fCost > y->fCost); + if (x->fCost == y->fCost) { + if (x->gCost == y->gCost) + // This is needed to guarantee that the sorting is stable. + return (x->nodeNum < y->nodeNum); + else + return (x->gCost < y->gCost); + } else + return (x->fCost > y->fCost); } }; diff --git a/rts/Sim/Path/QTPFS/PathThreads.h b/rts/Sim/Path/QTPFS/PathThreads.h index 62b4bdc212..f9a89ca089 100644 --- a/rts/Sim/Path/QTPFS/PathThreads.h +++ b/rts/Sim/Path/QTPFS/PathThreads.h @@ -90,11 +90,25 @@ namespace QTPFS { }; struct SearchQueueNode { - bool operator < (const SearchQueueNode& n) const { return (heapPriority < n.heapPriority); } - bool operator > (const SearchQueueNode& n) const { return (heapPriority > n.heapPriority); } - bool operator == (const SearchQueueNode& n) const { return (heapPriority == n.heapPriority); } - bool operator <= (const SearchQueueNode& n) const { return (heapPriority <= n.heapPriority); } - bool operator >= (const SearchQueueNode& n) const { return (heapPriority >= n.heapPriority); } + // Sorting Comparisons + // These need to guarantee stable ordering, even if the sorting algorithm itself is not stable. + bool operator < (const SearchQueueNode& n) const { + if (heapPriority == n.heapPriority) + return (nodeIndex > n.nodeIndex); + else + return (heapPriority < n.heapPriority); + } + bool operator > (const SearchQueueNode& n) const { + if (heapPriority == n.heapPriority) + return (nodeIndex < n.nodeIndex); + else + return (heapPriority > n.heapPriority); + } + + // General Comparisons + bool operator == (const SearchQueueNode& n) const { return (heapPriority == n.heapPriority); } + bool operator <= (const SearchQueueNode& n) const { return (heapPriority <= n.heapPriority); } + bool operator >= (const SearchQueueNode& n) const { return (heapPriority >= n.heapPriority); } SearchQueueNode(int index, float newPriorty) : heapPriority(newPriorty) From b423e08f6ee81b145f3756844914ebe466027abd Mon Sep 17 00:00:00 2001 From: lostsquirrel Date: Sun, 8 Dec 2024 20:09:43 +0000 Subject: [PATCH 2/7] fix indentation --- rts/Sim/Path/QTPFS/PathThreads.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/rts/Sim/Path/QTPFS/PathThreads.h b/rts/Sim/Path/QTPFS/PathThreads.h index f9a89ca089..155b8961a5 100644 --- a/rts/Sim/Path/QTPFS/PathThreads.h +++ b/rts/Sim/Path/QTPFS/PathThreads.h @@ -93,17 +93,17 @@ namespace QTPFS { // Sorting Comparisons // These need to guarantee stable ordering, even if the sorting algorithm itself is not stable. bool operator < (const SearchQueueNode& n) const { + if (heapPriority == n.heapPriority) + return (nodeIndex > n.nodeIndex); + else + return (heapPriority < n.heapPriority); + } + bool operator > (const SearchQueueNode& n) const { if (heapPriority == n.heapPriority) - return (nodeIndex > n.nodeIndex); - else - return (heapPriority < n.heapPriority); - } - bool operator > (const SearchQueueNode& n) const { - if (heapPriority == n.heapPriority) - return (nodeIndex < n.nodeIndex); - else - return (heapPriority > n.heapPriority); - } + return (nodeIndex < n.nodeIndex); + else + return (heapPriority > n.heapPriority); + } // General Comparisons bool operator == (const SearchQueueNode& n) const { return (heapPriority == n.heapPriority); } From b0392b4b69d4cf0dbc05bfbdf60ff69f22fe1259 Mon Sep 17 00:00:00 2001 From: lostsquirrel Date: Thu, 12 Dec 2024 08:59:52 +0000 Subject: [PATCH 3/7] Tidy up code to better communicate intent. --- rts/Sim/Path/QTPFS/PathThreads.h | 36 +++++++++++++------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/rts/Sim/Path/QTPFS/PathThreads.h b/rts/Sim/Path/QTPFS/PathThreads.h index 155b8961a5..a7072389ea 100644 --- a/rts/Sim/Path/QTPFS/PathThreads.h +++ b/rts/Sim/Path/QTPFS/PathThreads.h @@ -90,26 +90,6 @@ namespace QTPFS { }; struct SearchQueueNode { - // Sorting Comparisons - // These need to guarantee stable ordering, even if the sorting algorithm itself is not stable. - bool operator < (const SearchQueueNode& n) const { - if (heapPriority == n.heapPriority) - return (nodeIndex > n.nodeIndex); - else - return (heapPriority < n.heapPriority); - } - bool operator > (const SearchQueueNode& n) const { - if (heapPriority == n.heapPriority) - return (nodeIndex < n.nodeIndex); - else - return (heapPriority > n.heapPriority); - } - - // General Comparisons - bool operator == (const SearchQueueNode& n) const { return (heapPriority == n.heapPriority); } - bool operator <= (const SearchQueueNode& n) const { return (heapPriority <= n.heapPriority); } - bool operator >= (const SearchQueueNode& n) const { return (heapPriority >= n.heapPriority); } - SearchQueueNode(int index, float newPriorty) : heapPriority(newPriorty) , nodeIndex(index) @@ -119,9 +99,21 @@ namespace QTPFS { int nodeIndex; }; + /// Functor to define node priority. + /// Needs to guarantee stable ordering, even if the sorting algorithm itself is not stable. + struct ShouldMoveTowardsBottomOfPriorityQueue { + inline bool operator() (const SearchQueueNode& lhs, const SearchQueueNode& rhs) const { + if (lhs.heapPriority == rhs.heapPriority) + return (lhs.nodeIndex < rhs.nodeIndex); + else + return (lhs.heapPriority > rhs.heapPriority); + } + }; + + // Reminder that std::priority does comparisons to push element back to the bottom. So using - // std::greater here means the smallest value will be top() - typedef std::priority_queue, std::greater> SearchPriorityQueue; + // ShouldMoveTowardsBottomOfPriorityQueue here means the smallest value will be top() + typedef std::priority_queue, ShouldMoveTowardsBottomOfPriorityQueue> SearchPriorityQueue; struct SearchThreadData { From d3bcfce515d10aaa4e528e5a291fb85546bf9bc3 Mon Sep 17 00:00:00 2001 From: lostsquirrel Date: Thu, 12 Dec 2024 11:58:52 +0000 Subject: [PATCH 4/7] code clean up and compare gCost the same way as fCost --- rts/Sim/Path/HAPFS/PathDataTypes.h | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/rts/Sim/Path/HAPFS/PathDataTypes.h b/rts/Sim/Path/HAPFS/PathDataTypes.h index 909fb34c38..bd27617bb8 100644 --- a/rts/Sim/Path/HAPFS/PathDataTypes.h +++ b/rts/Sim/Path/HAPFS/PathDataTypes.h @@ -37,16 +37,10 @@ struct PathNode { /// functor to define node priority +/// This needs to guarantee that the sorting is stable. struct lessCost { inline bool operator() (const PathNode* x, const PathNode* y) const { - if (x->fCost == y->fCost) { - if (x->gCost == y->gCost) - // This is needed to guarantee that the sorting is stable. - return (x->nodeNum < y->nodeNum); - else - return (x->gCost < y->gCost); - } else - return (x->fCost > y->fCost); + return std::tie(x->fCost, x->gCost, x->nodeNum) > std::tie(y->fCost, y->gCost, y->nodeNum); } }; From 0551def096fa73afbc231a0857c633f4036dd718 Mon Sep 17 00:00:00 2001 From: lostsquirrel Date: Thu, 12 Dec 2024 12:04:21 +0000 Subject: [PATCH 5/7] applied std:tie treat to priority queue for QTPFS --- rts/Sim/Path/QTPFS/PathThreads.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rts/Sim/Path/QTPFS/PathThreads.h b/rts/Sim/Path/QTPFS/PathThreads.h index a7072389ea..9c6be8b543 100644 --- a/rts/Sim/Path/QTPFS/PathThreads.h +++ b/rts/Sim/Path/QTPFS/PathThreads.h @@ -103,10 +103,7 @@ namespace QTPFS { /// Needs to guarantee stable ordering, even if the sorting algorithm itself is not stable. struct ShouldMoveTowardsBottomOfPriorityQueue { inline bool operator() (const SearchQueueNode& lhs, const SearchQueueNode& rhs) const { - if (lhs.heapPriority == rhs.heapPriority) - return (lhs.nodeIndex < rhs.nodeIndex); - else - return (lhs.heapPriority > rhs.heapPriority); + return std::tie(lhs.heapPriority, lhs.nodeIndex) > std::tie(rhs.heapPriority, rhs.nodeIndex); } }; From a7c8883e997c73e14dc1067483d67aed5b7f9d15 Mon Sep 17 00:00:00 2001 From: lostsquirrel Date: Thu, 12 Dec 2024 13:18:22 +0000 Subject: [PATCH 6/7] prioritize smaller hCost over gCost. --- rts/Sim/Path/HAPFS/PathDataTypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rts/Sim/Path/HAPFS/PathDataTypes.h b/rts/Sim/Path/HAPFS/PathDataTypes.h index bd27617bb8..11b8c7463e 100644 --- a/rts/Sim/Path/HAPFS/PathDataTypes.h +++ b/rts/Sim/Path/HAPFS/PathDataTypes.h @@ -40,7 +40,7 @@ struct PathNode { /// This needs to guarantee that the sorting is stable. struct lessCost { inline bool operator() (const PathNode* x, const PathNode* y) const { - return std::tie(x->fCost, x->gCost, x->nodeNum) > std::tie(y->fCost, y->gCost, y->nodeNum); + return std::tie(x->fCost, y->gCost, x->nodeNum) > std::tie(y->fCost, x->gCost, y->nodeNum); } }; From 4028aa9190357bfa2b5a76bf3b2ad8be48f77758 Mon Sep 17 00:00:00 2001 From: lostsquirrel Date: Thu, 12 Dec 2024 13:25:05 +0000 Subject: [PATCH 7/7] add comment to avoid future confusion. --- rts/Sim/Path/HAPFS/PathDataTypes.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rts/Sim/Path/HAPFS/PathDataTypes.h b/rts/Sim/Path/HAPFS/PathDataTypes.h index 11b8c7463e..cf83b119c6 100644 --- a/rts/Sim/Path/HAPFS/PathDataTypes.h +++ b/rts/Sim/Path/HAPFS/PathDataTypes.h @@ -39,8 +39,11 @@ struct PathNode { /// functor to define node priority /// This needs to guarantee that the sorting is stable. struct lessCost { - inline bool operator() (const PathNode* x, const PathNode* y) const { - return std::tie(x->fCost, y->gCost, x->nodeNum) > std::tie(y->fCost, x->gCost, y->nodeNum); + inline bool operator() (const PathNode* lhs, const PathNode* rhs) const { + // fCost == gCost + hCost. + // When fCosts are the same, prioritize the node closest the goal. Since we don't have hCost to hand, we know + // that hCost == fCost - gCost. This is why we invert the left/right side for the gCost comparison. + return std::tie(lhs->fCost, rhs->gCost, lhs->nodeNum) > std::tie(rhs->fCost, lhs->gCost, rhs->nodeNum); } };