Merge pull request #837 from SeisSol/fix/cluster-merging

Fix off by one error in cluster merging
SeisSol · Apr 14, 2023 · 9b1b0ec · 9b1b0ec
2 parents 8e67533 + bc1e04b
commit 9b1b0ec
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 20 deletions.
diff --git a/src/Initializer/time_stepping/LtsWeights/LtsWeights.cpp b/src/Initializer/time_stepping/LtsWeights/LtsWeights.cpp
@@ -186,7 +186,7 @@ void LtsWeights::computeWeights(PUML::TETPUML const& mesh, double maximumAllowed
     wiggleFactor = wiggleFactorResult.wiggleFactor;
     if (ltsParameters->isAutoMergeUsed()) {
       maxClusterIdToEnforce =
-          std::min(maxClusterIdToEnforce, wiggleFactorResult.numberOfClusters - 1);
+          std::min(maxClusterIdToEnforce, wiggleFactorResult.maxClusterId);
     }
   } else {
     wiggleFactor = 1.0;
@@ -222,8 +222,8 @@ LtsWeights::ComputeWiggleFactorResult
   const auto rank = seissol::MPI::mpi.rank();
 
   // Maps that keep track of number of clusters vs cost
-  auto mapNumberOfClustersToLowestCost = std::map<int, double>{};
-  auto mapNumberOfClustersToBestWiggleFactor = std::map<int, double>{};
+  auto mapMaxClusterIdToLowestCost = std::map<int, double>{};
+  auto maxMapClusterIdToBestWiggleFactor = std::map<int, double>{};
 
   const double minWiggleFactor = ltsParameters->getWiggleFactorMinimum();
   const double maxWiggleFactor = 1.0;
@@ -296,49 +296,49 @@ LtsWeights::ComputeWiggleFactorResult
                                                       m_details.globalMinTimeStep,
                                                       MPI::mpi.comm());
 
-    if (auto it = mapNumberOfClustersToLowestCost.find(maxClusterId);
-        it == mapNumberOfClustersToLowestCost.end() || cost <= it->second) {
-      mapNumberOfClustersToBestWiggleFactor[maxClusterId] = curWiggleFactor;
-      mapNumberOfClustersToLowestCost[maxClusterId] = cost;
+    if (auto it = mapMaxClusterIdToLowestCost.find(maxClusterId);
+        it == mapMaxClusterIdToLowestCost.end() || cost <= it->second) {
+      maxMapClusterIdToBestWiggleFactor[maxClusterId] = curWiggleFactor;
+      mapMaxClusterIdToLowestCost[maxClusterId] = cost;
     }
   }
 
   // Find best wiggle factor after merging of clusters
   // We compare against cost of baselineCost.
-  int minAdmissibleNumberOfClusters = std::numeric_limits<int>::max();
+  int minAdmissibleMaxClusterId = std::numeric_limits<int>::max();
   if (isAutoMergeUsed) {
     // When merging clusters, we want to find the minimum number of clusters with admissible performance.
     bool foundAdmissibleMerge = false;
-    for (const auto& [noOfClusters, cost] : mapNumberOfClustersToLowestCost) {
+    for (const auto& [noOfClusters, cost] : mapMaxClusterIdToLowestCost) {
       if (cost <= maxAdmissibleCost) {
         foundAdmissibleMerge = true;
-        minAdmissibleNumberOfClusters = std::min(minAdmissibleNumberOfClusters, noOfClusters);
+        minAdmissibleMaxClusterId = std::min(minAdmissibleMaxClusterId, noOfClusters);
         logDebug(rank) << "Admissible. cluster:" << noOfClusters << ",cost" << cost
-                       << "with wiggle factor" << mapNumberOfClustersToBestWiggleFactor[noOfClusters];
+                       << "with wiggle factor" << maxMapClusterIdToBestWiggleFactor[noOfClusters];
       } else {
         logDebug(rank) << "Not admissible. cluster:" << noOfClusters << ",cost" << cost
-                       << "with wiggle factor" << mapNumberOfClustersToBestWiggleFactor[noOfClusters];
+                       << "with wiggle factor" << maxMapClusterIdToBestWiggleFactor[noOfClusters];
       }
     }
     if (!foundAdmissibleMerge) {
       logError() << "Found no admissible wiggle factor with cluster merge. Aborting.";
     }
   } else {
     // Otherwise choose one with the smallest cost
-    minAdmissibleNumberOfClusters =
-        std::min_element(mapNumberOfClustersToLowestCost.begin(),
-                         mapNumberOfClustersToLowestCost.end(),
+    minAdmissibleMaxClusterId =
+        std::min_element(mapMaxClusterIdToLowestCost.begin(),
+                         mapMaxClusterIdToLowestCost.end(),
                          [](const auto& a, const auto& b) { return a.second < b.second; })
             ->first;
   }
 
   logInfo(rank) << "Enforcing maximum difference when finding best wiggle factor took"
                 << totalWiggleFactorReductions << "reductions.";
 
-  const auto bestWiggleFactor = mapNumberOfClustersToBestWiggleFactor[minAdmissibleNumberOfClusters];
-  const auto bestCostEstimate = mapNumberOfClustersToLowestCost[minAdmissibleNumberOfClusters];
+  const auto bestWiggleFactor = maxMapClusterIdToBestWiggleFactor[minAdmissibleMaxClusterId];
+  const auto bestCostEstimate = mapMaxClusterIdToLowestCost[minAdmissibleMaxClusterId];
   logInfo(rank) << "The best wiggle factor is" << bestWiggleFactor << "with cost"
-                << bestCostEstimate << "and" << minAdmissibleNumberOfClusters << "time clusters";
+                << bestCostEstimate << "and" << minAdmissibleMaxClusterId + 1 << "time clusters";
 
   if (baselineCost > bestCostEstimate) {
     logInfo(rank) << "Cost decreased" << (*baselineCost - bestCostEstimate) / *baselineCost * 100
@@ -351,7 +351,7 @@ LtsWeights::ComputeWiggleFactorResult
     logInfo(rank) << "Note: Cost increased due to cluster merging!";
   }
 
-  return ComputeWiggleFactorResult{ minAdmissibleNumberOfClusters, bestWiggleFactor, bestCostEstimate};
+  return ComputeWiggleFactorResult{minAdmissibleMaxClusterId, bestWiggleFactor, bestCostEstimate};
 }
 
 const int* LtsWeights::vertexWeights() const {

diff --git a/src/Initializer/time_stepping/LtsWeights/LtsWeights.h b/src/Initializer/time_stepping/LtsWeights/LtsWeights.h
@@ -138,7 +138,7 @@ class LtsWeights {
   double wiggleFactor = 1.0;
   std::map<double, decltype(m_clusterIds)> clusteringCache; // Maps wiggle factor to clustering
   struct ComputeWiggleFactorResult {
-    int numberOfClusters;
+    int maxClusterId;
     double wiggleFactor;
     double cost;
   };