Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 3 additions & 2 deletions gtfs2ntfs/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,14 @@ fn run(opt: Opt) -> Result<()> {
let model = if opt.ignore_transfers {
model
} else {
generates_transfers(
let collections = generates_transfers(
model,
opt.max_distance,
opt.walking_speed,
opt.waiting_time,
None,
)?
)?;
transit_model::Model::new(collections)?
};

match opt.output.extension() {
Expand Down
2 changes: 1 addition & 1 deletion ntfs2ntfs/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn run(opt: Opt) -> Result<()> {

let model = transit_model::ntfs::read(opt.input)?;
let model = if opt.ignore_transfers {
model
model.into_collections()
} else {
generates_transfers(
model,
Expand Down
4 changes: 2 additions & 2 deletions src/ntfs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ where
/// [NTFS](https://github.com/hove-io/ntfs-specification/blob/master/ntfs_fr.md)
/// files in the given directory.
pub fn write<P: AsRef<path::Path>>(
model: &Model,
model: &Collections,
path: P,
current_datetime: DateTime<FixedOffset>,
) -> Result<()> {
Expand Down Expand Up @@ -437,7 +437,7 @@ pub fn write<P: AsRef<path::Path>>(
/// [NTFS](https://github.com/hove-io/ntfs-specification/blob/master/ntfs_fr.md)
/// ZIP archive at the given full path.
pub fn write_to_zip<P: AsRef<path::Path>>(
model: &Model,
model: &Collections,
path: P,
current_datetime: DateTime<FixedOffset>,
) -> Result<()> {
Expand Down
228 changes: 219 additions & 9 deletions src/transfers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
//! See function generates_transfers

use crate::{
model::Model,
model::{Collections, Model},
objects::{Coord, StopPoint, Transfer},
Result,
};
use std::collections::HashMap;
use std::time::Instant;
use tracing::info;
use typed_index_collection::{Collection, CollectionWithId, Idx};

Expand Down Expand Up @@ -49,7 +50,72 @@ pub fn get_available_transfers(
.collect()
}

/// Spatial grid to efficiently find nearby stop points
struct SpatialGrid {
/// Cell size in degrees (approximation)
cell_size: f64,
/// Map from (cell_x, cell_y) to list of stop_point_idx
cells: HashMap<(i32, i32), Vec<Idx<StopPoint>>>,
}

impl SpatialGrid {
/// Create a new spatial grid with cells sized to contain points within max_distance
/// We make cells larger (3x max_distance) to reduce the number of cells to check
fn new(max_distance: f64) -> Self {
// Approximate cell size in degrees (at equator, 1 degree ≈ 111km)
// Use 3x max_distance so we only need to check current cell + immediate neighbors
let cell_size = (max_distance * 3.0) / 111_000.0; // Convert meters to degrees
Self {
cell_size,
cells: HashMap::new(),
}
}

/// Get the cell coordinates for a given coordinate
#[inline]
fn get_cell(&self, coord: &Coord) -> (i32, i32) {
(
(coord.lon / self.cell_size).floor() as i32,
(coord.lat / self.cell_size).floor() as i32,
)
}

/// Insert a stop point into the grid
fn insert(&mut self, idx: Idx<StopPoint>, coord: &Coord) {
let cell = self.get_cell(coord);
self.cells.entry(cell).or_default().push(idx);
}

/// Fill the provided vector with stop point indices in the cell and adjacent cells (3x3 grid)
/// This reuses the Vec buffer to avoid allocations
#[inline]
fn get_nearby_indices_into(&self, coord: &Coord, result: &mut Vec<Idx<StopPoint>>) {
result.clear();
let (cell_x, cell_y) = self.get_cell(coord);

// Check the 9 cells: current + 8 adjacent
// Use saturating_add to avoid overflow with extreme coordinates
for dx in -1..=1 {
for dy in -1..=1 {
let target_x = cell_x.saturating_add(dx);
let target_y = cell_y.saturating_add(dy);
if let Some(indices) = self.cells.get(&(target_x, target_y)) {
result.extend_from_slice(indices);
}
}
}
}
}

/// Generate missing transfers from stop points within the required distance
///
/// This function uses a spatial grid optimization to avoid O(n²) complexity.
/// Instead of comparing every stop point with every other stop point, it:
/// 1. Divides the geographic space into a grid of cells
/// 2. For each stop point, only checks points in the same cell and adjacent cells (3x3 grid)
///
/// Complexity: O(n × k) where k is the average number of points per cell neighborhood
/// For uniformly distributed points, this is approximately O(n) instead of O(n²)
pub fn generate_missing_transfers_from_sp(
transfers_map: &TransferMap,
model: &Model,
Expand All @@ -58,18 +124,51 @@ pub fn generate_missing_transfers_from_sp(
waiting_time: u32,
need_transfer: Option<NeedTransfer>,
) -> TransferMap {
let total_start = Instant::now();
info!("Adding missing transfers from stop points.");
let mut new_transfers_map = TransferMap::new();
let sq_max_distance = max_distance * max_distance;

// Build spatial grid for efficient proximity queries
let grid_start = Instant::now();
let mut grid = SpatialGrid::new(max_distance);
let mut valid_stop_count = 0;
for (idx, sp) in model.stop_points.iter() {
if sp.coord != Coord::default() {
grid.insert(idx, &sp.coord);
valid_stop_count += 1;
}
}
let grid_duration = grid_start.elapsed();
info!(
"Built spatial grid with {} cells for {} valid stop points in {:.2?}",
grid.cells.len(),
valid_stop_count,
grid_duration
);

// Pre-allocate buffer for nearby indices to avoid repeated allocations
let mut nearby_indices = Vec::with_capacity(100);

let compute_start = Instant::now();
let mut total_comparisons = 0_u64;
let mut total_transfers_created = 0_u64;
let mut total_distance_checks = 0_u64;

// For each stop point, only check nearby points from the grid
for (idx1, sp1) in model.stop_points.iter() {
if sp1.coord == Coord::default() {
continue;
}
let approx = sp1.coord.approx();
for (idx2, sp2) in model.stop_points.iter() {
if sp2.coord == Coord::default() {
continue;
}

// Get nearby point indices (same cell + adjacent cells) - reuses the buffer
grid.get_nearby_indices_into(&sp1.coord, &mut nearby_indices);

// Only iterate over nearby points
for &idx2 in &nearby_indices {
total_comparisons += 1;

if transfers_map.contains_key(&(idx1, idx2)) {
continue;
}
Expand All @@ -78,10 +177,15 @@ pub fn generate_missing_transfers_from_sp(
continue;
}
}
let sp2 = &model.stop_points[idx2];

total_distance_checks += 1;
let sq_distance = approx.sq_distance_to(&sp2.coord);
if sq_distance > sq_max_distance {
continue;
}

total_transfers_created += 1;
let transfer_time = (sq_distance.sqrt() / walking_speed) as u32;
new_transfers_map.insert(
(idx1, idx2),
Expand All @@ -95,6 +199,22 @@ pub fn generate_missing_transfers_from_sp(
);
}
}

let compute_duration = compute_start.elapsed();
let total_duration = total_start.elapsed();

info!(
"Transfer computation stats: {} comparisons, {} distance checks, {} transfers created in {:.2?}",
total_comparisons,
total_distance_checks,
total_transfers_created,
compute_duration
);
info!(
"Total time for generate_missing_transfers_from_sp: {:.2?}",
total_duration
);

new_transfers_map
}

Expand Down Expand Up @@ -130,9 +250,19 @@ pub fn generates_transfers(
walking_speed: f64,
waiting_time: u32,
need_transfer: Option<NeedTransfer>,
) -> Result<Model> {
) -> Result<Collections> {
let total_start = Instant::now();
info!("Generating transfers...");

let get_transfers_start = Instant::now();
let mut transfers_map = get_available_transfers(model.transfers.clone(), &model.stop_points);
info!(
"get_available_transfers: {} existing transfers in {:.2?}",
transfers_map.len(),
get_transfers_start.elapsed()
);

let gen_transfers_start = Instant::now();
let new_transfers_map = generate_missing_transfers_from_sp(
&transfers_map,
&model,
Expand All @@ -141,16 +271,39 @@ pub fn generates_transfers(
waiting_time,
need_transfer,
);
info!(
"generate_missing_transfers_from_sp returned {} new transfers in {:.2?}",
new_transfers_map.len(),
gen_transfers_start.elapsed()
);

let merge_start = Instant::now();
transfers_map.extend(new_transfers_map);
let mut new_transfers: Vec<_> = transfers_map.into_values().collect();
info!("Merged transfers in {:.2?}", merge_start.elapsed());

let sort_start = Instant::now();
new_transfers.sort_unstable_by(|t1, t2| {
(&t1.from_stop_id, &t1.to_stop_id).cmp(&(&t2.from_stop_id, &t2.to_stop_id))
});
info!(
"Sorted {} transfers in {:.2?}",
new_transfers.len(),
sort_start.elapsed()
);

let rebuild_start = Instant::now();
let mut collections = model.into_collections();
collections.transfers = Collection::new(new_transfers);
Model::new(collections)
// let result = Model::new(collections);
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

This commented-out code should be removed rather than left in the codebase as it serves no purpose and adds clutter.

Suggested change
// let result = Model::new(collections);

Copilot uses AI. Check for mistakes.
info!("Rebuilt model in {:.2?}", rebuild_start.elapsed());

info!(
"generates_transfers TOTAL TIME: {:.2?}",
total_start.elapsed()
);

Ok(collections)
}

#[cfg(test)]
Expand Down Expand Up @@ -361,8 +514,8 @@ mod tests {
#[test]
fn test_generates_transfers() {
let model = base_model();
let new_model = generates_transfers(model, 100.0, 0.7, 2, None).expect("an error occured");
let mut collections = new_model.into_collections();
let mut collections =
generates_transfers(model, 100.0, 0.7, 2, None).expect("an error occured");

let mut transfers = Collection::new(vec![
Transfer {
Expand Down Expand Up @@ -441,4 +594,61 @@ mod tests {

assert_eq!(transfers, transfers_expected);
}

#[test]
fn test_spatial_grid_performance() {
// Create a model with many stop points to demonstrate the performance improvement
let mut model_builder = ModelBuilder::default();

// Create a grid of stop points (e.g., 100 x 100 = 10,000 points)
// In a real scenario with 10k points:
// - O(n²) = 100,000,000 comparisons
// - O(n×k) with spatial grid ≈ 10,000 × 9 cells × ~10 points = ~900,000 comparisons
// This is roughly 100x faster!

let grid_size = 10; // Use 10x10 = 100 points for the test (to keep it fast)
for i in 0..grid_size {
for j in 0..grid_size {
let stop_id = format!("SP_{i}_{j}");
// Create stops in a 0.01° x 0.01° grid (roughly 1km x 1km)

model_builder = model_builder.vj(&format!("vj_{i}_{j}"), |vj_builder| {
vj_builder
.route(&format!("route_{i}_{j}"))
.st(&stop_id, "10:00:00");
});
}
}

let transit_model = model_builder.build();
let mut collections = transit_model.into_collections();

// Set coordinates for all stop points
for i in 0..grid_size {
for j in 0..grid_size {
let stop_id = format!("SP_{i}_{j}");
collections.stop_points.get_mut(&stop_id).unwrap().coord = Coord {
lon: 2.39 + (i as f64) * 0.001,
lat: 48.85 + (j as f64) * 0.001,
};
}
}

let model = Model::new(collections).unwrap();

// Generate transfers with a reasonable distance (500m)
let result = generates_transfers(model, 500.0, 0.7, 2, None);
assert!(result.is_ok());

// With spatial grid, this should complete quickly even with 100+ points
// The number of transfers should be reasonable (not n²)
let new_model = result.unwrap();
let transfer_count = new_model.transfers.len();
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The variable is named new_model but the function now returns Collections, not Model. This causes a type mismatch - new_model should be renamed to collections or similar to match the actual type.

Suggested change
let new_model = result.unwrap();
let transfer_count = new_model.transfers.len();
let collections = result.unwrap();
let transfer_count = collections.transfers.len();

Copilot uses AI. Check for mistakes.

// Each point should have transfers to nearby points (not all points)
// With 100 points in a 10x10 grid and 500m max distance,
// each point should connect to roughly 4-9 neighbors
assert!(transfer_count < grid_size * grid_size * grid_size * grid_size);
assert!(transfer_count > 0);
}
}