Express all pathfinding costs in units of seconds. #82, #494 (#587)

This is simpler to reason about, allows the penalty for entering a zone
or taking an unprotected turn to be expressed in terms of a time
penalty, and is a step towards adjusting bike/foot routing for elevation
data.

When we later add things like "safety/quietness" for cycling, maybe we
can switch to using a (time, quietness) tuple, and transform into a
single number with a linear combination parameterized by that agent's
preference for time/safety. This change is compatible with that future
idea.

There are behavior changes here, particularly for zones and unprotected
turns. No new maps start gridlocking, and in fact, Rainier starts
working again.

game/src/challenges/prebake.rs

@@ -30,7 +30,7 @@ pub fn prebake_all() {
         MapName::seattle("lakeslice"),
         MapName::seattle("phinney"),
         MapName::seattle("qa"),
-        //MapName::seattle("rainier_valley"),   // TODO broken
+        MapName::seattle("rainier_valley"),
         MapName::seattle("wallingford"),
     ] {
         let map = map_model::Map::load_synchronously(name.path(), &mut timer);

game/src/debug/routes.rs

@@ -1,7 +1,7 @@
 use std::collections::HashMap;
 
 use abstutil::{prettyprint_usize, Counter, Parallelism, Timer};
-use geom::Polygon;
+use geom::{Duration, Polygon};
 use map_gui::colors::ColorSchemeChoice;
 use map_gui::tools::ColorNetwork;
 use map_gui::{AppLike, ID};
@@ -208,7 +208,7 @@ fn params_to_controls(ctx: &mut EventCtx, mode: TripMode, params: &RoutingParams
             Spinner::widget(
                 ctx,
                 (1, 100),
-                (params.unprotected_turn_penalty * 10.0) as isize,
+                params.unprotected_turn_penalty.inner_seconds() as isize,
             )
             .named("unprotected turn penalty"),
         ]));
@@ -237,13 +237,15 @@ fn params_to_controls(ctx: &mut EventCtx, mode: TripMode, params: &RoutingParams
 fn controls_to_params(panel: &Panel) -> (TripMode, RoutingParams) {
     let mut params = RoutingParams::default();
     if !panel.is_button_enabled("cars") {
-        params.unprotected_turn_penalty = panel.spinner("unprotected turn penalty") as f64 / 10.0;
+        params.unprotected_turn_penalty =
+            Duration::seconds(panel.spinner("unprotected turn penalty") as f64);
         return (TripMode::Drive, params);
     }
     if !panel.is_button_enabled("pedestrians") {
         return (TripMode::Walk, params);
     }
-    params.unprotected_turn_penalty = panel.spinner("unprotected turn penalty") as f64 / 10.0;
+    params.unprotected_turn_penalty =
+        Duration::seconds(panel.spinner("unprotected turn penalty") as f64 / 10.0);
     params.bike_lane_penalty = panel.spinner("bike lane penalty") as f64 / 10.0;
     params.bus_lane_penalty = panel.spinner("bus lane penalty") as f64 / 10.0;
     params.driving_lane_penalty = panel.spinner("driving lane penalty") as f64 / 10.0;
@@ -479,7 +481,7 @@ fn cmp_count(after: usize, before: usize) -> Vec<TextSpan> {
 /// one start.
 pub struct PathCostDebugger {
     draw_path: Drawable,
-    costs: HashMap<RoadID, f64>,
+    costs: HashMap<RoadID, Duration>,
     tooltip: Option<Text>,
     panel: Panel,
 }
@@ -516,8 +518,11 @@ impl State<App> for PathCostDebugger {
         if ctx.redo_mouseover() {
             self.tooltip = None;
             if let Some(ID::Road(r)) = app.mouseover_unzoomed_roads_and_intersections(ctx) {
-                let cost = self.costs.get(&r).cloned().unwrap_or(-1.0);
+                if let Some(cost) = self.costs.get(&r) {
                     self.tooltip = Some(Text::from(format!("Cost: {}", cost)));
+                } else {
+                    self.tooltip = Some(Text::from("No cost"));
+                }
             }
         }
 

game/src/debug/uber_turns.rs

@@ -1,6 +1,6 @@
 use std::collections::BTreeSet;
 
-use geom::ArrowCap;
+use geom::{ArrowCap, Duration};
 use map_gui::render::{DrawOptions, BIG_ARROW_THICKNESS};
 use map_gui::tools::PopupMsg;
 use map_gui::ID;
@@ -152,7 +152,7 @@ impl UberTurnViewer {
         for i in &ic.members {
             batch.push(Color::BLUE.alpha(0.5), map.get_i(*i).polygon.clone());
         }
-        let mut sum_cost = 0.0;
+        let mut sum_cost = Duration::ZERO;
         if !ic.uber_turns.is_empty() {
             let ut = &ic.uber_turns[idx];
             batch.push(

map_model/src/connectivity/mod.rs

@@ -4,7 +4,7 @@ use std::collections::{HashMap, HashSet};
 
 use petgraph::graphmap::DiGraphMap;
 
-use geom::{Distance, Duration, Speed};
+use geom::Duration;
 
 pub use self::walking::{all_walking_costs_from, WalkingOptions};
 use crate::pathfind::{build_graph_for_vehicles, zone_cost};
@@ -77,9 +77,6 @@ pub fn all_vehicle_costs_from(
         }
     }
 
-    // TODO Copied from simulation code :(
-    let max_bike_speed = Speed::miles_per_hour(10.0);
-
     if let Some(start_lane) = bldg_to_lane.get(&start) {
         let graph = build_graph_for_vehicles(map, constraints);
         let cost_per_lane = petgraph::algo::dijkstra(&graph, *start_lane, None, |(_, _, turn)| {
@@ -92,9 +89,7 @@ pub fn all_vehicle_costs_from(
             )
         });
         for (b, lane) in bldg_to_lane {
-            if let Some(meters) = cost_per_lane.get(&lane) {
+            if let Some(duration) = cost_per_lane.get(&lane).cloned() {
-                let distance = Distance::meters(*meters as f64);
-                let duration = distance / max_bike_speed;
                 if duration <= time_limit {
                     results.insert(b, duration);
                 }
@@ -106,7 +101,10 @@ pub fn all_vehicle_costs_from(
 }
 
 // TODO Refactor with all_vehicle_costs_from
-pub fn debug_vehicle_costs(req: PathRequest, map: &Map) -> Option<(f64, HashMap<RoadID, f64>)> {
+pub fn debug_vehicle_costs(
+    req: PathRequest,
+    map: &Map,
+) -> Option<(Duration, HashMap<RoadID, Duration>)> {
     // TODO Support this
     if req.constraints == PathConstraints::Pedestrian {
         return None;
@@ -127,7 +125,7 @@ pub fn debug_vehicle_costs(req: PathRequest, map: &Map) -> Option<(f64, HashMap<
                 map,
             ) + zone_cost(turn, req.constraints, map)
         },
-        |_| 0.0,
+        |_| Duration::ZERO,
     )?;
 
     let lane_costs = petgraph::algo::dijkstra(&graph, req.start.lane(), None, |(_, _, t)| {
@@ -145,7 +143,7 @@ pub fn debug_vehicle_costs(req: PathRequest, map: &Map) -> Option<(f64, HashMap<
     let mut road_costs = HashMap::new();
     for (l, cost) in lane_costs {
         let road_cost = road_costs.entry(map.get_l(l).parent).or_insert(cost);
-        *road_cost = road_cost.min(cost);
+        *road_cost = (*road_cost).min(cost);
     }
 
     Some((cost, road_costs))

map_model/src/pathfind/ch.rs

@@ -4,6 +4,7 @@
 use serde::{Deserialize, Serialize};
 
 use abstutil::Timer;
+use geom::Duration;
 
 use crate::pathfind::vehicles::VehiclePathfinder;
 use crate::pathfind::walking::{SidewalkPathfinder, WalkingNode};
@@ -108,3 +109,8 @@ impl ContractionHierarchyPathfinder {
         timer.stop("apply edits to pedestrian using transit pathfinding");
     }
 }
+
+pub fn round(cost: Duration) -> usize {
+    // Round up! 0 cost edges are ignored
+    (cost.inner_seconds().round() as usize).max(1)
+}

map_model/src/pathfind/dijkstra.rs

@@ -4,6 +4,8 @@ use std::collections::BTreeSet;
 
 use petgraph::graphmap::DiGraphMap;
 
+use geom::Duration;
+
 use crate::pathfind::vehicles::vehicle_cost;
 use crate::pathfind::walking::{walking_cost, WalkingNode};
 use crate::pathfind::zone_cost;
@@ -64,7 +66,7 @@ fn calc_path(
             vehicle_cost(map.get_l(turn.id.src), turn, req.constraints, params, map)
                 + zone_cost(turn, req.constraints, map)
         },
-        |_| 0.0,
+        |_| Duration::ZERO,
     )?;
 
     let mut steps = Vec::new();
@@ -82,8 +84,8 @@ fn calc_path(
     Some(Path::new(map, steps, req.clone(), Vec::new()))
 }
 
-pub fn build_graph_for_pedestrians(map: &Map) -> DiGraphMap<WalkingNode, usize> {
+pub fn build_graph_for_pedestrians(map: &Map) -> DiGraphMap<WalkingNode, Duration> {
-    let mut graph: DiGraphMap<WalkingNode, usize> = DiGraphMap::new();
+    let mut graph: DiGraphMap<WalkingNode, Duration> = DiGraphMap::new();
     for l in map.all_lanes() {
         if l.is_walkable() {
             let cost = walking_cost(l.length());
@@ -100,7 +102,7 @@ pub fn build_graph_for_pedestrians(map: &Map) -> DiGraphMap<WalkingNode, usize>
                         map.get_l(turn.id.dst).dst_i == turn.id.parent,
                     ),
                     walking_cost(turn.geom.length())
-                        + zone_cost(turn, PathConstraints::Pedestrian, map) as usize,
+                        + zone_cost(turn, PathConstraints::Pedestrian, map),
                 );
             }
         }
@@ -118,7 +120,7 @@ pub fn simple_walking_path(req: &PathRequest, map: &Map) -> Option<Vec<WalkingNo
         closest_start,
         |end| end == closest_end,
         |(_, _, cost)| *cost,
-        |_| 0,
+        |_| Duration::ZERO,
     )?;
     Some(path)
 }

map_model/src/pathfind/mod.rs

@@ -659,7 +659,7 @@ fn validate_zones(map: &Map, steps: &Vec<PathStep>, req: &PathRequest) {
 }
 
 /// Heavily penalize crossing into an access-restricted zone that doesn't allow this mode.
-pub fn zone_cost(turn: &Turn, constraints: PathConstraints, map: &Map) -> f64 {
+pub fn zone_cost(turn: &Turn, constraints: PathConstraints, map: &Map) -> Duration {
     // Detect when we cross into a new zone that doesn't allow constraints.
     if map
         .get_parent(turn.id.src)
@@ -672,11 +672,12 @@ pub fn zone_cost(turn: &Turn, constraints: PathConstraints, map: &Map) -> f64 {
             .allow_through_traffic
             .contains(constraints)
     {
-        // TODO Tune this after making vehicles_cost and walking_cost both roughly represent
+        // This should be high enough to achieve the desired effect of somebody not entering
-        // seconds. In the meantime, this penalty seems high enough to achieve the desired effect.
+        // the zone unless absolutely necessary. Someone would violate that and cut through anyway
-        100_000.0
+        // only when the alternative route would take more than 3 hours longer!
+        Duration::hours(3)
     } else {
-        0.0
+        Duration::ZERO
     }
 }
 
@@ -685,9 +686,10 @@ pub fn zone_cost(turn: &Turn, constraints: PathConstraints, map: &Map) -> f64 {
 // space-expensive change right now.
 #[derive(PartialEq, Serialize, Deserialize)]
 pub struct RoutingParams {
-    // For all vehicles
+    // For all vehicles. This is added to the cost of a movement as an additional delay.
-    pub unprotected_turn_penalty: f64,
+    pub unprotected_turn_penalty: Duration,
-    // For bike routing
+    // For bike routing. Multiplied by the base cost, since spending more time on the wrong lane
+    // type matters.
     pub bike_lane_penalty: f64,
     pub bus_lane_penalty: f64,
     pub driving_lane_penalty: f64,
@@ -696,7 +698,9 @@ pub struct RoutingParams {
 impl RoutingParams {
     pub const fn default() -> RoutingParams {
         RoutingParams {
-            unprotected_turn_penalty: 2.0,
+            // This is a total guess -- it really depends on the traffic patterns of the particular
+            // road at the time we're routing.
+            unprotected_turn_penalty: Duration::const_seconds(30.0),
             bike_lane_penalty: 1.0,
             bus_lane_penalty: 1.1,
             driving_lane_penalty: 1.5,

map_model/src/pathfind/vehicles.rs

@@ -7,7 +7,9 @@ use serde::{Deserialize, Serialize};
 use thread_local::ThreadLocal;
 
 use abstutil::MultiMap;
+use geom::{Duration, Speed};
 
+use crate::pathfind::ch::round;
 use crate::pathfind::node_map::{deserialize_nodemap, NodeMap};
 use crate::pathfind::uber_turns::{IntersectionCluster, UberTurn};
 use crate::pathfind::zone_cost;
@@ -187,7 +189,7 @@ fn make_input_graph(
                     any = true;
                     let ut = &uber_turns[*idx];
 
-                    let mut sum_cost = 0.0;
+                    let mut sum_cost = Duration::ZERO;
                     for t in &ut.path {
                         let turn = map.get_t(*t);
                         sum_cost += vehicle_cost(
@@ -234,32 +236,36 @@ fn make_input_graph(
     input_graph
 }
 
-/// Different unit based on constraints.
+/// This returns the pathfinding cost of crossing one lane and turn. This is also expressed in
+/// units of time. It factors in the ideal time to cross the space, along with penalties for
+/// entering an access-restricted zone, taking an unprotected turn, and so on.
 pub fn vehicle_cost(
     lane: &Lane,
     turn: &Turn,
     constraints: PathConstraints,
     params: &RoutingParams,
     map: &Map,
-) -> f64 {
+) -> Duration {
-    // TODO Could cost turns differently.
-
     let base = match constraints {
         PathConstraints::Car | PathConstraints::Train => {
-            // Prefer slightly longer route on faster roads
             let t1 = lane.length() / map.get_r(lane.parent).speed_limit;
             let t2 = turn.geom.length() / map.get_parent(turn.id.dst).speed_limit;
-            (t1 + t2).inner_seconds()
+            t1 + t2
         }
         PathConstraints::Bike => {
-            // Speed limits don't matter, bikes are usually constrained by their own speed limit.
+            // TODO Copied from sim. Probably move to map_model.
-            let dist = lane.length() + turn.geom.length();
+            let max_bike_speed = Speed::miles_per_hour(10.0);
+            // Usually the bike's speed limit matters, not the road's.
+            let t1 = lane.length() / map.get_r(lane.parent).speed_limit.min(max_bike_speed);
+            let t2 =
+                turn.geom.length() / map.get_parent(turn.id.dst).speed_limit.min(max_bike_speed);
+
             // TODO Elevation gain is bad, loss is good.
             // TODO If we're on a driving lane, higher speed limit is worse.
             // TODO Bike lanes next to parking is dangerous.
 
-            // TODO Prefer bike lanes, then bus lanes, then driving lanes. For now, express that as
+            // TODO Prefer bike lanes, then bus lanes, then driving lanes. For now, express that by
-            // an extra cost.
+            // multiplying the base cost.
             let lt_penalty = if lane.is_biking() {
                 params.bike_lane_penalty
             } else if lane.is_bus() {
@@ -269,8 +275,7 @@ pub fn vehicle_cost(
                 params.driving_lane_penalty
             };
 
-            // 1m resolution is fine
+            lt_penalty * (t1 + t2)
-            (lt_penalty * dist).inner_meters()
         }
         PathConstraints::Bus => {
             // Like Car, but prefer bus lanes.
@@ -282,7 +287,7 @@ pub fn vehicle_cost(
                 assert!(lane.is_driving());
                 1.1
             };
-            (lt_penalty * (t1 + t2)).inner_seconds()
+            lt_penalty * (t1 + t2)
         }
         PathConstraints::Pedestrian => unreachable!(),
     };
@@ -299,7 +304,7 @@ pub fn vehicle_cost(
         && rank_from < rank_to
         && map.get_i(turn.id.parent).is_stop_sign()
     {
-        base * params.unprotected_turn_penalty
+        base + params.unprotected_turn_penalty
     } else {
         base
     };
@@ -313,11 +318,8 @@ pub fn vehicle_cost(
     if constraints == PathConstraints::Bike {
         extra_penalty = slow_lane;
     }
+    // TODO These are small integers, just treat them as seconds for now to micro-adjust the
+    // specific choice of lane.
 
-    base + (extra_penalty as f64)
+    base + Duration::seconds(extra_penalty as f64)
-}
-
-// Round up! 0 cost edges are ignored
-fn round(cost: f64) -> usize {
-    (cost.round() as usize).max(1)
 }

map_model/src/pathfind/walking.rs

@@ -8,8 +8,9 @@ use fast_paths::{deserialize_32, serialize_32, FastGraph, InputGraph, PathCalcul
 use serde::{Deserialize, Serialize};
 use thread_local::ThreadLocal;
 
-use geom::{Distance, Speed};
+use geom::{Distance, Duration, Speed};
 
+use crate::pathfind::ch::round;
 use crate::pathfind::node_map::{deserialize_nodemap, NodeMap};
 use crate::pathfind::vehicles::VehiclePathfinder;
 use crate::pathfind::zone_cost;
@@ -237,12 +238,12 @@ fn make_input_graph(
             let mut cost = walking_cost(l.length());
             // TODO Tune this penalty, along with many others.
             if l.is_shoulder() {
-                cost *= 2;
+                cost = 2.0 * cost;
             }
             let n1 = nodes.get(WalkingNode::SidewalkEndpoint(l.id, true));
             let n2 = nodes.get(WalkingNode::SidewalkEndpoint(l.id, false));
-            input_graph.add_edge(n1, n2, cost);
+            input_graph.add_edge(n1, n2, round(cost));
-            input_graph.add_edge(n2, n1, cost);
+            input_graph.add_edge(n2, n1, round(cost));
         }
     }
 
@@ -255,8 +256,9 @@ fn make_input_graph(
             input_graph.add_edge(
                 nodes.get(from),
                 nodes.get(to),
-                walking_cost(t.geom.length())
+                round(
-                    + zone_cost(t, PathConstraints::Pedestrian, map) as usize,
+                    walking_cost(t.geom.length()) + zone_cost(t, PathConstraints::Pedestrian, map),
+                ),
             );
         }
     }
@@ -286,12 +288,12 @@ fn transit_input_graph(
             } else {
                 walking_cost(stop.sidewalk_pos.dist_along())
             };
-            // Add some extra penalty (equivalent to 1m) to using a bus stop. Otherwise a path
+            // Add some extra penalty to using a bus stop. Otherwise a path might try to pass
-            // might try to pass through it uselessly.
+            // through it uselessly.
-            let penalty = 100;
+            let penalty = Duration::seconds(1.0);
             let sidewalk = nodes.get(WalkingNode::SidewalkEndpoint(lane.id, *endpt));
-            input_graph.add_edge(sidewalk, ride_bus, cost + penalty);
+            input_graph.add_edge(sidewalk, ride_bus, round(cost + penalty));
-            input_graph.add_edge(ride_bus, sidewalk, cost + penalty);
+            input_graph.add_edge(ride_bus, sidewalk, round(cost + penalty));
         }
     }
 
@@ -380,12 +382,10 @@ fn transit_input_graph(
     }
 }
 
-/// The cost is time in seconds, rounded to a usize
 // TODO Plumb RoutingParams here, but first, need to also plumb in the turn or lane.
-pub fn walking_cost(dist: Distance) -> usize {
+pub fn walking_cost(dist: Distance) -> Duration {
     let walking_speed = Speed::meters_per_second(1.34);
-    let time = dist / walking_speed;
+    dist / walking_speed
-    (time.inner_seconds().round() as usize).max(1)
 }
 
 pub fn walking_path_to_steps(path: Vec<WalkingNode>, map: &Map) -> Vec<PathStep> {