Skip to content

routing: remove largest-SCC filtering as a hard default #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blackopsrepl merged 1 commit into from
Mar 21, 2026
Merged

routing: remove largest-SCC filtering as a hard default #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub mod routing;

pub use geometry::{decode_polyline, encode_polyline, EncodedSegment};
pub use routing::{
haversine_distance, BBoxError, BoundingBox, CacheStats, Coord, CoordError, NetworkConfig,
NetworkRef, Objective, RoadNetwork, RouteResult, RoutingError, RoutingProgress, RoutingResult,
SnappedCoord, SpeedProfile, TravelTimeMatrix, UNREACHABLE,
haversine_distance, BBoxError, BoundingBox, CacheStats, ConnectivityPolicy, Coord, CoordError,
NetworkConfig, NetworkRef, Objective, RoadNetwork, RouteResult, RoutingError, RoutingProgress,
RoutingResult, SnappedCoord, SpeedProfile, TravelTimeMatrix, UNREACHABLE,
};
13 changes: 13 additions & 0 deletions src/routing/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
use std::path::PathBuf;
use std::time::Duration;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ConnectivityPolicy {
KeepAll,
LargestStronglyConnectedComponent,
}

#[derive(Debug, Clone)]
pub struct SpeedProfile {
pub motorway: f64,
Expand Down Expand Up @@ -92,6 +98,7 @@ pub struct NetworkConfig {
pub connect_timeout: Duration,
pub read_timeout: Duration,
pub speed_profile: SpeedProfile,
pub connectivity_policy: ConnectivityPolicy,
pub highway_types: Vec<&'static str>,
}

Expand All @@ -103,6 +110,7 @@ impl Default for NetworkConfig {
connect_timeout: Duration::from_secs(30),
read_timeout: Duration::from_secs(180),
speed_profile: SpeedProfile::default(),
connectivity_policy: ConnectivityPolicy::KeepAll,
highway_types: vec![
"motorway",
"trunk",
Expand Down Expand Up @@ -148,6 +156,11 @@ impl NetworkConfig {
self
}

pub fn connectivity_policy(mut self, policy: ConnectivityPolicy) -> Self {
self.connectivity_policy = policy;
self
}

pub fn highway_types(mut self, types: Vec<&'static str>) -> Self {
self.highway_types = types;
self
Expand Down
76 changes: 49 additions & 27 deletions src/routing/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::cache::{
cache, record_hit, record_miss, CachedEdge, CachedNetwork, CachedNode, NetworkRef,
CACHE_VERSION,
};
use super::config::NetworkConfig;
use super::config::{ConnectivityPolicy, NetworkConfig};
use super::coord::Coord;
use super::error::RoutingError;
use super::network::{EdgeData, RoadNetwork};
Expand Down Expand Up @@ -60,7 +60,7 @@ impl RoadNetwork {
if let Some(tx) = progress {
let _ = tx.send(RoutingProgress::CheckingCache { percent: 8 }).await;
}
match Self::load_from_file(&cache_path).await {
match Self::load_from_file(&cache_path, config).await {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Key the in-memory cache by connectivity policy

This only makes the file-cache load path policy-aware. RoadNetwork::load_or_fetch still returns an in-memory hit before reaching this call, and that cache is keyed only by bbox.cache_key(). If one process loads the same bbox first with ConnectivityPolicy::KeepAll and later with LargestStronglyConnectedComponent (or vice versa), the second call silently reuses the first graph, so routes and matrices are computed with the wrong connectivity semantics.

Useful? React with 👍 / 👎.

Ok(n) => {
if let Some(tx) = progress {
let _ = tx
Expand Down Expand Up @@ -314,27 +314,38 @@ out body;"#,
way_count
);

// Filter to largest strongly connected component to ensure all nodes are reachable
let scc_count = network.strongly_connected_components();
if scc_count > 1 {
info!(
"Road network has {} SCCs, filtering to largest component",
scc_count
);
network.filter_to_largest_scc();
info!(
"After SCC filter: {} nodes, {} edges",
network.node_count(),
network.edge_count()
);
match config.connectivity_policy {
ConnectivityPolicy::KeepAll => {
if scc_count > 1 {
info!(
"Road network has {} SCCs, preserving all components by configuration",
scc_count
);
}
}
ConnectivityPolicy::LargestStronglyConnectedComponent => {
if scc_count > 1 {
info!(
"Road network has {} SCCs, filtering to largest component",
scc_count
);
network.filter_to_largest_scc();
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid persisting SCC-filtered graphs as canonical cache files

Filtering the freshly fetched graph here mutates it before load_or_fetch writes it to the shared <bbox>.json cache file. In the scenario where a bbox is first populated with ConnectivityPolicy::LargestStronglyConnectedComponent, every later default KeepAll load—even in a fresh process—can only reload the already-pruned graph, so the new default no longer preserves disconnected components for that region.

Useful? React with 👍 / 👎.

info!(
"After SCC filter: {} nodes, {} edges",
network.node_count(),
network.edge_count()
);
}
}
}

network.build_spatial_index();

Ok(network)
}

async fn load_from_file(path: &Path) -> Result<Self, RoutingError> {
async fn load_from_file(path: &Path, config: &NetworkConfig) -> Result<Self, RoutingError> {
let data = tokio::fs::read_to_string(path).await?;

let cached: CachedNetwork = match serde_json::from_str(&data) {
Expand Down Expand Up @@ -365,19 +376,30 @@ out body;"#,
network.add_edge_by_index(edge.from, edge.to, edge.travel_time_s, edge.distance_m);
}

// Filter to largest SCC (cached networks from older versions may not be filtered)
let scc_count = network.strongly_connected_components();
if scc_count > 1 {
info!(
"Cached network has {} SCCs, filtering to largest component",
scc_count
);
network.filter_to_largest_scc();
info!(
"After SCC filter: {} nodes, {} edges",
network.node_count(),
network.edge_count()
);
match config.connectivity_policy {
ConnectivityPolicy::KeepAll => {
if scc_count > 1 {
info!(
"Cached network has {} SCCs, preserving all components by configuration",
scc_count
);
}
}
ConnectivityPolicy::LargestStronglyConnectedComponent => {
if scc_count > 1 {
info!(
"Cached network has {} SCCs, filtering to largest component",
scc_count
);
network.filter_to_largest_scc();
info!(
"After SCC filter: {} nodes, {} edges",
network.node_count(),
network.edge_count()
);
}
}
}

network.build_spatial_index();
Expand Down
2 changes: 1 addition & 1 deletion src/routing/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ mod spatial;

pub use bbox::BoundingBox;
pub use cache::{CacheStats, NetworkRef};
pub use config::{NetworkConfig, SpeedProfile};
pub use config::{ConnectivityPolicy, NetworkConfig, SpeedProfile};
pub use coord::Coord;
pub use error::{BBoxError, CoordError, RoutingError};
pub use matrix::{TravelTimeMatrix, UNREACHABLE};
Expand Down
39 changes: 36 additions & 3 deletions tests/integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ use std::path::PathBuf;
use std::time::Duration;

use solverforge_maps::{
decode_polyline, encode_polyline, haversine_distance, BBoxError, BoundingBox, Coord,
CoordError, NetworkConfig, RoadNetwork, RouteResult, RoutingError, SpeedProfile, UNREACHABLE,
decode_polyline, encode_polyline, haversine_distance, BBoxError, BoundingBox,
ConnectivityPolicy, Coord, CoordError, NetworkConfig, RoadNetwork, RouteResult, RoutingError,
SpeedProfile, UNREACHABLE,
};

mod types {
Expand Down Expand Up @@ -140,11 +141,16 @@ mod types {
let config = NetworkConfig::new()
.overpass_url("https://custom.api/interpreter")
.cache_dir("/tmp/cache")
.connect_timeout(Duration::from_secs(60));
.connect_timeout(Duration::from_secs(60))
.connectivity_policy(ConnectivityPolicy::LargestStronglyConnectedComponent);

assert_eq!(config.overpass_url, "https://custom.api/interpreter");
assert_eq!(config.cache_dir, PathBuf::from("/tmp/cache"));
assert_eq!(config.connect_timeout, Duration::from_secs(60));
assert_eq!(
config.connectivity_policy,
ConnectivityPolicy::LargestStronglyConnectedComponent
);
}

#[test]
Expand All @@ -156,6 +162,12 @@ mod types {
let maxspeed_mps = profile.speed_mps(Some("50"), "motorway");
assert!((maxspeed_mps - 13.889).abs() < 0.1);
}

#[test]
fn default_connectivity_policy_keeps_all_components() {
let config = NetworkConfig::default();
assert_eq!(config.connectivity_policy, ConnectivityPolicy::KeepAll);
}
}
}

Expand Down Expand Up @@ -242,6 +254,27 @@ mod routing {
assert_eq!(network.strongly_connected_components(), 0);
assert!((network.largest_component_fraction() - 0.0).abs() < f64::EPSILON);
}

#[test]
fn largest_scc_filter_is_opt_in() {
let mut network = RoadNetwork::from_test_data(
&[(0.0, 0.0), (0.0, 1.0), (10.0, 10.0), (10.0, 11.0)],
&[
(0, 1, 10.0, 100.0),
(1, 0, 10.0, 100.0),
(2, 3, 10.0, 100.0),
],
);

assert_eq!(network.node_count(), 4);
assert_eq!(network.strongly_connected_components(), 3);

network.filter_to_largest_scc();

assert_eq!(network.node_count(), 2);
assert_eq!(network.edge_count(), 2);
assert_eq!(network.strongly_connected_components(), 1);
}
}

mod route_simplify {
Expand Down
Loading