Merge pull request #117 from NatLabRockies/yep/lakewood_tests

Improvements to island detection and performance of OMF import

Author: yamilbknsu

rust/bambam-omf/src/app/network.rs

@@ -1,6 +1,6 @@
-use std::path::Path;
+use std::{collections::HashSet, path::Path, sync::Arc};
 
-use geo::{Contains, Geometry};
+use geo::{BoundingRect, Geometry, Intersects};
 use rayon::prelude::*;
 use routee_compass_core::model::unit::DistanceUnit;
 use serde::{Deserialize, Serialize};
@@ -135,31 +135,53 @@ fn run_collector(
 }
 
 /// filters segments and connectors in a transportation collection
-/// using an arbitrary extent with the `contains` predicate. empty geometries are ignored (filtered out)
+/// using an arbitrary extent with the `intersect` predicate over the segments collection.
+/// a second pass is performed over the connectors collection to reomve all connectors not referenced
+/// by the remaining segments.
+/// empty geometries are ignored (filtered out).
 fn apply_extent_to_collection(
     collection: TransportationCollection,
     extent: Geometry<f32>,
 ) -> TransportationCollection {
-    let filtered_segments = collection
+    log::info!("Started applying extent to segments");
+    let extent_arc = Arc::new(extent);
+    let filtered_segments: Vec<crate::collection::TransportationSegmentRecord> = collection
         .segments
         .into_par_iter()
-        .filter(|segment| {
-            segment
-                .get_linestring()
-                .map(|linestring| extent.contains(linestring))
-                .unwrap_or(false)
+        .filter(|segment| match segment.get_linestring() {
+            Ok(ls) => {
+                let Some(bbox) = ls.bounding_rect() else {
+                    return false;
+                };
+
+                // Short-circuit condition for bbox
+                extent_arc.intersects(&bbox) && extent_arc.intersects(ls)
+            }
+            Err(_) => false,
         })
         .collect();
 
+    log::info!("Collecting all connector IDs");
+    let connector_ids = filtered_segments
+        .par_iter()
+        .flat_map(|segment| {
+            segment
+                .connectors
+                .as_ref()
+                .unwrap_or(&vec![])
+                .iter()
+                .map(|con_ref| con_ref.connector_id.clone())
+                .collect::<Vec<String>>()
+        })
+        .collect::<HashSet<String>>();
+
+    let arc_ids = Arc::new(connector_ids);
+
+    log::info!("Started applying extent to connectors");
     let filtered_connectors = collection
         .connectors
         .into_par_iter()
-        .filter(|connector| {
-            connector
-                .get_geometry()
-                .map(|geometry| extent.contains(geometry))
-                .unwrap_or(false)
-        })
+        .filter(|connector| arc_ids.contains(&connector.id))
         .collect();
 
     TransportationCollection {

rust/bambam-omf/src/graph/component_algorithm.rs

@@ -94,21 +94,22 @@ pub fn island_detection_algorithm(
                     component.push((current_el_id, current_e_id));
                     let current_edge = edge_lists[current_el_id.0].0[current_e_id.0];
 
-                    let current_distance = is_component_island_sequential(
+                    // Update the max_distance
+                    let current_midpoint = compute_midpoint(&current_edge, vertices);
+                    let current_distance =
+                        Haversine.length(&line_string![start_midpoint.0, current_midpoint.0]);
+                    let current_distance_uom =
+                        uom_length::new::<uom::si::length::meter>(current_distance as f64);
+                    max_distance_reached = max_distance_reached.max(current_distance_uom);
+
+                    visit_edge(
                         &current_edge,
-                        start_midpoint,
                         &mut visited,
                         &mut queue,
-                        vertices,
                         &forward_adjacency,
                         &backward_adjacency,
                     );
 
-                    // Update the max_distance
-                    let current_distance_uom =
-                        uom_length::new::<uom::si::length::meter>(current_distance as f64);
-                    max_distance_reached = max_distance_reached.max(current_distance_uom);
-
                     // Update bar
                     if let Err(e) = pb.update(1) {
                         log::warn!("error during update of progress bar: {e}")
@@ -131,42 +132,49 @@ pub fn island_detection_algorithm(
     island_edges
 }
 
-/// returns the f32 distance in meters from the current edge midpoint to the initial edge midpoint.
-fn is_component_island_sequential(
+/// visit operation for weakly-connected BFS traversal
+fn visit_edge(
     edge: &Edge,
-    initial_midpoint: Point<f32>,
     visited: &mut HashSet<(EdgeListId, EdgeId)>,
     queue: &mut VecDeque<(EdgeListId, EdgeId)>,
-    vertices: &[Vertex],
     forward_adjacency: &DenseAdjacencyList,
     backward_adjacency: &DenseAdjacencyList,
-) -> f32 {
+) -> () {
     let (edge_list_id, edge_id) = (edge.edge_list_id, edge.edge_id);
 
-    // Update counter
-    let current_midpoint = compute_midpoint(edge, vertices);
-    let current_distance_to_start_meters =
-        Haversine.length(&line_string![initial_midpoint.0, current_midpoint.0]);
-
     // get all neighbors, add them to queue
+    // forward_adjacency[dst]: edges leaving dst (v → *)
     let outward_edges: Vec<&(EdgeListId, EdgeId)> =
         forward_adjacency[edge.dst_vertex_id.0].keys().collect();
     for (edge_list_id, edge_id) in outward_edges {
         queue.push_back((*edge_list_id, *edge_id));
     }
+    // backward_adjacency[src]: edges entering src (* → u)
     let inward_edges: Vec<&(EdgeListId, EdgeId)> =
         backward_adjacency[edge.src_vertex_id.0].keys().collect();
     for (edge_list_id, edge_id) in inward_edges {
         queue.push_back((*edge_list_id, *edge_id));
     }
+    // forward_adjacency[src]: other edges leaving src (u → *) — catches pure source vertices
+    let sibling_outward_edges: Vec<&(EdgeListId, EdgeId)> =
+        forward_adjacency[edge.src_vertex_id.0].keys().collect();
+    for (edge_list_id, edge_id) in sibling_outward_edges {
+        queue.push_back((*edge_list_id, *edge_id));
+    }
+    // backward_adjacency[dst]: other edges entering dst (* → v) — catches pure sink vertices
+    let sibling_inward_edges: Vec<&(EdgeListId, EdgeId)> =
+        backward_adjacency[edge.dst_vertex_id.0].keys().collect();
+    for (edge_list_id, edge_id) in sibling_inward_edges {
+        queue.push_back((*edge_list_id, *edge_id));
+    }
 
     // mark as visited
     visited.insert((edge_list_id, edge_id));
-
-    current_distance_to_start_meters
 }
 
-/// parallelizable implementation
+/// parallelizable implementation. Explores the entire component this
+/// edge belongs to up to a given distance threshold and returns whether or
+/// not the component is an island
 fn is_component_island_parallel(
     edge: &Edge,
     distance_threshold: f64,
@@ -176,9 +184,9 @@ fn is_component_island_parallel(
     forward_adjacency: &DenseAdjacencyList,
     backward_adjacency: &DenseAdjacencyList,
 ) -> Result<bool, OvertureMapsCollectionError> {
-    let mut visited = HashSet::<(&EdgeListId, &EdgeId)>::new();
-    let mut visit_queue: VecDeque<(&EdgeListId, &EdgeId)> = VecDeque::new();
-    visit_queue.push_back((&edge.edge_list_id, &edge.edge_id));
+    let mut visited = HashSet::<(EdgeListId, EdgeId)>::new();
+    let mut visit_queue: VecDeque<(EdgeListId, EdgeId)> = VecDeque::new();
+    visit_queue.push_back((edge.edge_list_id, edge.edge_id));
 
     let edge_midpoint = compute_midpoint(edge, vertices);
     let mut max_distance_reached = uom_length::new::<uom::si::length::meter>(0 as f64);
@@ -193,28 +201,20 @@ fn is_component_island_parallel(
             {
                 continue;
             }
-            visited.insert((current_edge_list_id, current_edge_id));
 
             // Retrieve current edge information
             let current_edge = edge_lists.get(current_edge_list_id.0)
-                                                            .and_then(|el| el.get(current_edge_id))
-                                                            .ok_or(OvertureMapsCollectionError::InternalError(format!("edge list {current_edge_list_id:?} or edge {current_edge_id:?} not found during island detection starting at edge {edge:?}")))?;
+                .and_then(|el| el.get(&current_edge_id))
+                .ok_or(OvertureMapsCollectionError::InternalError(format!("edge list {current_edge_list_id:?} or edge {current_edge_id:?} not found during island detection starting at edge {edge:?}")))?;
 
             // Expand queue
-            let outward_edges: Vec<&(EdgeListId, EdgeId)> = forward_adjacency
-                [current_edge.dst_vertex_id.0]
-                .keys()
-                .collect();
-            for (edge_list_id, edge_id) in outward_edges {
-                visit_queue.push_back((edge_list_id, edge_id));
-            }
-            let inward_edges: Vec<&(EdgeListId, EdgeId)> = backward_adjacency
-                [current_edge.src_vertex_id.0]
-                .keys()
-                .collect();
-            for (edge_list_id, edge_id) in inward_edges {
-                visit_queue.push_back((edge_list_id, edge_id));
-            }
+            visit_edge(
+                &current_edge,
+                &mut visited,
+                &mut visit_queue,
+                forward_adjacency,
+                backward_adjacency,
+            );
 
             // Update counter
             let current_midpoint = compute_midpoint(current_edge, vertices);

rust/bambam-omf/src/graph/omf_graph.rs

@@ -11,8 +11,10 @@ use crate::{
         SegmentFullType, TransportationCollection, TransportationSegmentRecord,
     },
     graph::{
-        component_algorithm::island_detection_algorithm, segment_ops,
-        serialize_ops::clean_omf_edge_list, vertex_serializable::VertexSerializable,
+        component_algorithm::island_detection_algorithm,
+        segment_ops,
+        serialize_ops::{clean_omf_edge_list, compute_vertex_remapping},
+        vertex_serializable::VertexSerializable,
         OmfGraphSummary,
     },
 };
@@ -61,9 +63,11 @@ impl OmfGraphVectorized {
         island_detection_configuration: Option<IslandDetectionAlgorithmConfiguration>,
     ) -> Result<Self, OvertureMapsCollectionError> {
         // process all connectors into vertices
+        log::info!("Creating vertex lookup");
         let (mut vertices, mut vertex_lookup) =
             ops::create_vertices_and_lookup(&collection.connectors, None)?;
 
+        log::info!("Processing edge lists");
         // for each mode configuration, create an edge list
         let mut edge_lists: Vec<OmfEdgeList> = vec![];
         for (index, edge_list_config) in configuration.iter().enumerate() {
@@ -73,6 +77,7 @@ impl OmfGraphVectorized {
             let mut filter = edge_list_config.filter.clone();
             filter.sort(); // sort for performance
 
+            log::info!("Filtering edge list {edge_list_id}");
             // filter to the segments that match our travel mode filter(s)
             let segments: Vec<&TransportationSegmentRecord> = collection
                 .segments
@@ -84,6 +89,7 @@ impl OmfGraphVectorized {
             // the splits are locations in each segment record where we want to define a vertex
             // which may not yet exist on the graph. this is where we begin to impose directivity
             // in our records.
+            log::info!("Creating splits");
             let mut splits = vec![];
             for heading in [SegmentHeading::Forward, SegmentHeading::Backward] {
                 let mut when: SegmentAccessRestrictionWhen = edge_list_config.into();
@@ -99,6 +105,7 @@ impl OmfGraphVectorized {
 
             // depending on the split method, we may need to create additional vertices at locations
             // which are not OvertureMaps-defined connector types.
+            log::info!("Extending vertices");
             ops::extend_vertices(
                 &splits,
                 &segments,
@@ -108,6 +115,7 @@ impl OmfGraphVectorized {
             )?;
 
             // create all edges based on the above split points using all vertices.
+            log::info!("Creating edges");
             let edges = ops::create_edges(
                 &segments,
                 &segment_lookup,
@@ -116,11 +124,16 @@ impl OmfGraphVectorized {
                 &vertex_lookup,
                 edge_list_id,
             )?;
+            log::info!("Creating geometries");
             let geometries = ops::create_geometries(&segments, &segment_lookup, &splits)?;
+            log::info!("Creating bearings");
             let bearings = ops::bearing_deg_from_geometries(&geometries)?;
+            log::info!("Creating classes");
             let classes = ops::create_segment_full_types(&segments, &segment_lookup, &splits)?;
 
+            log::info!("Creating speeds");
             let speeds = ops::create_speeds(&segments, &segment_lookup, &splits)?;
+            log::info!("Creating speed lookup");
             let speed_lookup = ops::create_speed_by_segment_type_lookup(
                 &speeds,
                 &segments,
@@ -130,13 +143,16 @@ impl OmfGraphVectorized {
             )?;
 
             // insert global speed value for reference
+            log::info!("Computing global speed");
             let global_speed =
                 ops::get_global_average_speed(&speeds, &segments, &segment_lookup, &splits)?;
 
             // omf ids
+            log::info!("Computing omf_ids");
             let omf_segment_ids = ops::get_segment_omf_ids(&segments, &segment_lookup, &splits)?;
 
             // match speeds according to classes
+            log::info!("Completing speeds vector with default global");
             let speeds = speeds
                 .into_par_iter()
                 .zip(&classes)
@@ -172,6 +188,7 @@ impl OmfGraphVectorized {
         }
 
         // Compute islands in resulting edge lists and remove island edges
+        log::info!("Compute islands");
         if let Some(algorithm_config) = island_detection_configuration {
             let ref_edge_lists = edge_lists
                 .iter()
@@ -187,11 +204,31 @@ impl OmfGraphVectorized {
 
             // Refactor Vec into Hashmap
             let mut edges_lookup: HashMap<EdgeListId, Vec<EdgeId>> = HashMap::new();
-            for (a, b) in island_edges {
-                edges_lookup.entry(a).or_default().push(b);
+            for (a, b) in &island_edges {
+                edges_lookup.entry(*a).or_default().push(*b);
+            }
+
+            // Compute and apply vertex remapping
+            let vertex_remapping = compute_vertex_remapping(&vertices, &edge_lists, &island_edges)?;
+            vertices = vertices
+                .into_iter()
+                .filter_map(|vertex| {
+                    vertex_remapping[vertex.vertex_id.0].map(|vertex_id| Vertex {
+                        vertex_id,
+                        ..vertex
+                    })
+                })
+                .collect();
+
+            // dropping entries for removed vertices.
+            vertex_lookup.retain(|_, v| vertex_remapping[*v].is_some());
+            // Update vertex_lookup to reflect the remapped vertex indices,
+            for v in vertex_lookup.values_mut() {
+                *v = vertex_remapping[*v].ok_or(OvertureMapsCollectionError::InternalError(format!("vertex index {v} expected after island computation but was flagged for deletion")))?.0;
             }
 
             // Clean the edge lists
+            log::info!("Apply islands algorithm result");
             edge_lists = edge_lists
                 .into_iter()
                 .map(|omf_list| {
@@ -209,9 +246,9 @@ impl OmfGraphVectorized {
                         .map(|edge| !edges_to_remove.contains(&edge.edge_id))
                         .collect::<Vec<bool>>();
 
-                    clean_omf_edge_list(omf_list, mask)
+                    clean_omf_edge_list(omf_list, mask, &vertex_remapping)
                 })
-                .collect::<Vec<OmfEdgeList>>();
+                .collect::<Result<Vec<OmfEdgeList>, OvertureMapsCollectionError>>()?;
         };
 
         let result = Self {