primary vs perimeter

backend/benches/build_map_model.rs

@@ -9,18 +9,14 @@ fn benchmark_build_map_model(c: &mut Criterion) {
     ] {
         // Do the file i/o (reading OSM.xml) outside of the bench loop
         let map_model_builder = neighbourhood.map_model_builder().unwrap();
-        c.bench_function(
-            &format!(
-                "build map_model: {name}",
-                name = neighbourhood.study_area_name
-            ),
-            |b| {
+        c.benchmark_group(neighbourhood.savefile_name)
+            .sample_size(neighbourhood.bench_sample_size())
+            .bench_function("build map_model", |b| {
                 b.iter(|| {
                     let map_model = map_model_builder().unwrap();
                     black_box(map_model);
                 });
-            },
-        );
+            });
     }
 }
 

backend/benches/build_neighbourhood.rs

@@ -9,24 +9,15 @@ fn benchmark_build_neighbourhood(c: &mut Criterion) {
         NeighbourhoodFixture::STRASBOURG,
     ] {
         let (neighbourhood_boundary, map) = neighbourhood.neighbourhood_params().unwrap();
-        let edit_perimeter_roads = false;
-        c.bench_function(
-            &format!(
-                "build neighbourhood: {name}",
-                name = neighbourhood.savefile_name
-            ),
-            |b| {
+        c.benchmark_group(neighbourhood.savefile_name)
+            .sample_size(neighbourhood.bench_sample_size())
+            .bench_function("build neighbourhood", |b| {
                 b.iter(|| {
-                    let neighbourhood = Neighbourhood::new(
-                        &map,
-                        neighbourhood_boundary.clone(),
-                        edit_perimeter_roads,
-                    )
-                    .unwrap();
+                    let neighbourhood =
+                        Neighbourhood::new(&map, neighbourhood_boundary.clone()).unwrap();
                     black_box(neighbourhood);
                 });
-            },
-        );
+            });
     }
 }
 

backend/benches/router.rs

@@ -11,15 +11,15 @@ fn benchmark_build_router(c: &mut Criterion) {
         let map = neighbourhood.map_model().unwrap();
         let routing_input_before = map.router_input_before();
         let main_road_penalty = 1.0;
-        c.bench_function(
-            &format!("build router: {name}", name = neighbourhood.savefile_name),
-            |b| {
+
+        c.benchmark_group(neighbourhood.savefile_name)
+            .sample_size(neighbourhood.bench_sample_size())
+            .bench_function("build router", |b| {
                 b.iter(|| {
                     let router = Router::new(&routing_input_before, main_road_penalty);
                     black_box(router);
                 });
-            },
-        );
+            });
     }
 }
 
@@ -33,9 +33,9 @@ fn benchmark_route(c: &mut Criterion) {
         let router = Router::new(&map.router_input_before(), main_road_penalty);
 
         let route_requests = synthetic_od_requests(&map);
-        c.bench_function(
-            &format!("routing in {name}", name = neighbourhood.savefile_name),
-            |b| {
+        c.benchmark_group(neighbourhood.savefile_name)
+            .sample_size(neighbourhood.bench_sample_size())
+            .bench_function("routing", |b| {
                 b.iter(|| {
                     let mut num_found = 0;
                     for (start, end, _) in &route_requests {
@@ -47,15 +47,14 @@ fn benchmark_route(c: &mut Criterion) {
                         // These exact numbers are brittle - but they should only change if the
                         // routing logic or the input data are updated, and even then they shouldn't
                         // change by much.
-                        NeighbourhoodFixture::BRISTOL_EAST => assert_eq!(num_found, 842),
-                        NeighbourhoodFixture::STRASBOURG => assert_eq!(num_found, 902),
+                        NeighbourhoodFixture::BRISTOL_EAST => assert_eq!(num_found, 830),
+                        NeighbourhoodFixture::STRASBOURG => assert_eq!(num_found, 885),
                         _ => todo!(
                             "unknown neighbourhood: {neighbourhood:?}, (num_found: {num_found})"
                         ),
                     }
                 });
-            },
-        );
+            });
     }
 }
 

backend/src/auto_boundaries.rs

@@ -15,21 +15,7 @@ impl MapModel {
         let mut road_severances = Vec::new();
 
         for road in &self.roads {
-            if road.tags.is_any(
-                "highway",
-                vec![
-                    "motorway",
-                    "motorway_link",
-                    "trunk",
-                    "trunk_link",
-                    "primary",
-                    "primary_link",
-                    "secondary",
-                    "secondary_link",
-                    "tertiary",
-                    "tertiary_link",
-                ],
-            ) {
+            if road.is_severance() {
                 severances.push(road.linestring.clone());
                 road_severances.push(road.linestring.clone());
             }

backend/src/geo_helpers/mod.rs

@@ -3,8 +3,9 @@ pub use slice_nearest_boundary::SliceNearestFrechetBoundary;
 
 use geo::line_measures::InterpolatableLine;
 use geo::{
-    BooleanOps, BoundingRect, Contains, Coord, Distance, Euclidean, Intersects, Length, Line,
-    LineIntersection, LineLocatePoint, LineString, MultiPolygon, Point, Polygon, Rect, Validation,
+    BooleanOps, BoundingRect, Buffer, Contains, Coord, Distance, Euclidean, Intersects, Length,
+    Line, LineIntersection, LineLocatePoint, LineString, MultiPolygon, Point, Polygon, Rect,
+    Validation,
 };
 use rstar::AABB;
 use utils::LineSplit;
@@ -94,6 +95,37 @@ pub fn buffer_aabb(aabb: AABB<Point>, buffer_meters: f64) -> AABB<Point> {
     )
 }
 
+/// Buffers a polygon, returning the largest of the output Polygons
+///
+/// Buffering can leave floating artifacts.
+/// I haven't investigated why yet, but dealing with it is simple enough.
+/// i_overlay offers a relevant sounding `min_area` parameter, but it's not currently exposed
+/// by geo's buffer integration.
+pub fn buffer_polygon(polygon: &Polygon, distance: f64) -> anyhow::Result<Polygon> {
+    let merged = polygon.buffer(distance);
+
+    use geo::Area;
+    let area_polygons = merged.0.into_iter().map(|p| (p.unsigned_area(), p));
+
+    // Anything smaller than this should be considered an artifact and discarded.
+    //
+    // We may have to tweak this if we encounter errors with the current value, which is on the
+    // order of "house sized" - much smaller than any expected neighbourhood.
+    let buffering_artifact_threshold_m2 = 1000.;
+    let mut merged_boundaries: Vec<_> = area_polygons
+        .filter(|(area, _polygon)| *area > buffering_artifact_threshold_m2)
+        .collect();
+
+    let geometry = match merged_boundaries.len() {
+        0 => bail!("Empty boundary"),
+        1 => merged_boundaries.pop().expect("verified non-empty").1,
+        _ => {
+            bail!("All included boundaries must be adjacent");
+        }
+    };
+    Ok(geometry)
+}
+
 // TODO What in the generics is going on here...
 pub fn aabb<G: BoundingRect<f64, Output = Option<Rect<f64>>>>(geom: &G) -> AABB<Point> {
     let bbox: Rect = geom.bounding_rect().unwrap().into();

backend/src/lib.rs

@@ -217,11 +217,7 @@ impl LTN {
     }
 
     #[wasm_bindgen(js_name = setCurrentNeighbourhood)]
-    pub fn set_current_neighbourhood(
-        &mut self,
-        name: String,
-        edit_perimeter_roads: bool,
-    ) -> Result<(), JsValue> {
+    pub fn set_current_neighbourhood(&mut self, name: String) -> Result<(), JsValue> {
         let boundary = self.map.boundaries.get(&name).unwrap();
 
         // Are we still editing the same neighbourhood, just switching edit_perimeter_roads?
@@ -230,10 +226,8 @@ impl LTN {
             .as_ref()
             .map(|n| n.name() == name)
             .unwrap_or(false);
-        self.neighbourhood = Some(
-            Neighbourhood::new(&self.map, boundary.clone(), edit_perimeter_roads)
-                .map_err(err_to_js)?,
-        );
+        self.neighbourhood =
+            Some(Neighbourhood::new(&self.map, boundary.clone()).map_err(err_to_js)?);
 
         // Undoing edits in another neighbourhood doesn't make sense
         if !editing_same {

backend/src/map_model.rs

@@ -111,6 +111,28 @@ impl fmt::Debug for Road {
     }
 }
 
+impl Road {
+    pub fn is_severance(&self) -> bool {
+        // PERF: is_any/has_any should take a const slice, not an owned vec... though maybe
+        // the compiler is smart enough to optimize this.
+        self.tags.is_any(
+            "highway",
+            vec![
+                "motorway",
+                "motorway_link",
+                "trunk",
+                "trunk_link",
+                "primary",
+                "primary_link",
+                "secondary",
+                "secondary_link",
+                "tertiary",
+                "tertiary_link",
+            ],
+        )
+    }
+}
+
 /// Connection between `Road` (segments).
 #[derive(Debug, Clone)]
 pub struct Intersection {