Skip to content

Conversation

@datanel
Copy link
Member

@datanel datanel commented Oct 21, 2025

  • Using a spatial grid algorithm to avoid traversing n² stop points
  • Do not use Model::new to avoid time-consuming indexing.

@datanel datanel force-pushed the transfers/spatial_grid branch from f65e920 to aa341c5 Compare October 21, 2025 06:39
@datanel datanel force-pushed the transfers/spatial_grid branch from aa341c5 to 6b58bb3 Compare October 24, 2025 13:36
@datanel datanel requested a review from patochectp October 24, 2025 13:42
@datanel datanel requested a review from Copilot November 4, 2025 13:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes the transfer generation algorithm from O(n²) to O(n×k) complexity using a spatial grid data structure. The function signature changes from returning Model to returning Collections, requiring callers to rebuild the model if needed.

  • Introduces a SpatialGrid structure to partition geographic space and enable efficient proximity queries
  • Adds comprehensive performance logging to track grid construction, comparison counts, and timing metrics
  • Changes generates_transfers return type from Result<Model> to Result<Collections>

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/transfers.rs Implements spatial grid optimization, adds performance metrics, changes return type to Collections
src/ntfs/mod.rs Updates write function signatures to accept Collections instead of Model
ntfs2ntfs/src/main.rs Adjusts caller to handle Collections return type
gtfs2ntfs/src/main.rs Adjusts caller to rebuild Model from returned Collections

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/transfers.rs Outdated
Comment on lines 645 to 646
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants