change costmatrix max_distance threshold to a distance threshold inst…

…ead of duration (valhalla#4674)

CHANGELOG.md

@@ -113,6 +113,7 @@
    * CHANGED: CostMatrix switched from Dijkstra to A* [#4650](https://github.com/valhalla/valhalla/pull/4650)
    * ADDED: some missing documentation about request parameters [#4687](https://github.com/valhalla/valhalla/pull/4687)
    * ADDED: Consider more forward/backward tags for access restrictions and speeds [#4686](https://github.com/valhalla/valhalla/pull/4686)
+   * CHANGED: change costmatrix max_distance threshold to a distance threshold instead of duration [#4672](https://github.com/valhalla/valhalla/pull/4672)
 
 ## Release Date: 2023-05-11 Valhalla 3.4.0
 * **Removed**

src/thor/costmatrix.cc

@@ -57,32 +57,12 @@ CostMatrix::CostMatrix(const boost::property_tree::ptree& config)
       check_reverse_connections_(config.get<bool>("costmatrix_check_reverse_connection", false)),
       access_mode_(kAutoAccess),
       mode_(travel_mode_t::kDrive), locs_count_{0, 0}, locs_remaining_{0, 0},
-      current_cost_threshold_(0), targets_{new ReachedMap}, sources_{new ReachedMap} {
+      current_pathdist_threshold_(0), targets_{new ReachedMap}, sources_{new ReachedMap} {
 }
 
 CostMatrix::~CostMatrix() {
 }
 
-float CostMatrix::GetCostThreshold(const float max_matrix_distance) {
-  float cost_threshold;
-  switch (mode_) {
-    case travel_mode_t::kBicycle:
-      cost_threshold = max_matrix_distance / kCostThresholdBicycleDivisor;
-      break;
-    case travel_mode_t::kPedestrian:
-    case travel_mode_t::kPublicTransit:
-      cost_threshold = max_matrix_distance / kCostThresholdPedestrianDivisor;
-      break;
-    case travel_mode_t::kDrive:
-    default:
-      cost_threshold = max_matrix_distance / kCostThresholdAutoDivisor;
-  }
-
-  // Increase the cost threshold to make sure requests near the max distance succeed.
-  // Some costing models and locations require higher thresholds to succeed.
-  return cost_threshold * 2.0f;
-}
-
 // Clear the temporary information generated during time + distance matrix
 // construction.
 void CostMatrix::Clear() {
@@ -147,7 +127,7 @@ bool CostMatrix::SourceToTarget(Api& request,
   auto& source_location_list = *request.mutable_options()->mutable_sources();
   auto& target_location_list = *request.mutable_options()->mutable_targets();
 
-  current_cost_threshold_ = GetCostThreshold(max_matrix_distance);
+  current_pathdist_threshold_ = max_matrix_distance / 2;
 
   auto time_infos = SetOriginTimes(source_location_list, graphreader);
 
@@ -364,6 +344,9 @@ void CostMatrix::Initialize(
         auto heuristic = astar_heuristics_[!is_fwd][i].Get({other_ll.lng(), other_ll.lat()});
         min_heuristic = std::min(min_heuristic, heuristic);
       }
+      // TODO(nils): previously we'd estimate the bucket range by the max matrix distance,
+      // which would lead to tons of RAM if a high value was chosen in the config; ideally
+      // this would be chosen based on the request (e.g. some factor to the A* distance)
       adjacency_[is_fwd][i].reuse(min_heuristic, range, bucketsize, &edgelabel_[is_fwd][i]);
     }
   }
@@ -619,7 +602,7 @@ bool CostMatrix::Expand(const uint32_t index,
 
   // Get edge label and check cost threshold
   auto pred = edgelabels[pred_idx];
-  if (pred.cost().secs > current_cost_threshold_) {
+  if (pred.path_distance() > current_pathdist_threshold_) {
     locs_status_[FORWARD][index].threshold = 0;
     return false;
   }

test/matrix.cc

@@ -231,7 +231,7 @@ TEST(Matrix, test_matrix) {
 
   CostMatrix cost_matrix_abort_source;
   cost_matrix_abort_source.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive,
-                                          90000.0);
+                                          7000.0);
 
   matrix = request.matrix();
   uint32_t found = 0;
@@ -245,7 +245,7 @@ TEST(Matrix, test_matrix) {
 
   CostMatrix cost_matrix_abort_target;
   cost_matrix_abort_target.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive,
-                                          50000.0);
+                                          5000.0);
 
   matrix = request.matrix();
   found = 0;
@@ -254,7 +254,7 @@ TEST(Matrix, test_matrix) {
       ++found;
     }
   }
-  EXPECT_EQ(found, 11) << " not the number of results as expected";
+  EXPECT_EQ(found, 13) << " not the number of results as expected";
   request.clear_matrix();
 
   TimeDistanceMatrix timedist_matrix;

valhalla/thor/costmatrix.h

@@ -140,8 +140,8 @@ class CostMatrix : public MatrixAlgorithm {
   std::array<uint32_t, 2> locs_count_;
   std::array<uint32_t, 2> locs_remaining_;
 
-  // The cost threshold being used for the currently executing query
-  float current_cost_threshold_;
+  // The path distance threshold being used for the currently executing query
+  float current_pathdist_threshold_;
 
   // Status
   std::array<std::vector<LocationStatus>, 2> locs_status_;
@@ -163,13 +163,6 @@ class CostMatrix : public MatrixAlgorithm {
   // when doing timezone differencing a timezone cache speeds up the computation
   baldr::DateTime::tz_sys_info_cache_t tz_cache_;
 
-  /**
-   * Get the cost threshold based on the current mode and the max arc-length distance
-   * for that mode.
-   * @param  max_matrix_distance   Maximum arc-length distance for current mode.
-   */
-  float GetCostThreshold(const float max_matrix_distance);
-
   /**
    * Form the initial time distance matrix given the sources
    * and destinations.