Conversation
There was a problem hiding this comment.
Pull request overview
Renames the core search structure from SearchTree to SearchGraph to reflect that the search space is no longer guaranteed to be a strict tree after introducing LabelModel (aligning terminology with upcoming publication language).
Changes:
- Replaced
SearchTree/SearchTreeNode/SearchTreeErrorwithSearchGraph/SearchGraphNode/SearchGraphErroracross core search APIs and dependent crates. - Updated traversal/cost/termination/model interfaces and implementations to accept
&SearchGraph. - Updated output plugins and docs to consume/emit “graph” results (while some legacy “tree” naming remains in outputs).
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/routee-compass/src/plugin/output/default/traversal/traversal_output_format.rs | Accept SearchGraph for traversal output generation |
| rust/routee-compass/src/plugin/output/default/traversal/traversal_ops.rs | Traversal output helpers updated to use SearchGraph |
| rust/routee-compass/src/plugin/output/default/traversal/plugin.rs | Traversal plugin reads result.graphs |
| rust/routee-compass/src/plugin/output/default/summary/plugin.rs | Summary plugin counts edges from result.graphs |
| rust/routee-compass/src/plugin/output/default/eval/ops.rs | Removed unused test helper in eval ops tests |
| rust/routee-compass/src/app/search/search_app.rs | Maps algorithm trees into app-level graphs |
| rust/routee-compass/src/app/search/search_app_result.rs | Renamed result field to graphs: Vec<SearchGraph> |
| rust/routee-compass/src/app/compass/compass_app_ops.rs | Updated empty result construction to graphs |
| rust/routee-compass-powertrain/src/model/phev_energy_model.rs | Traversal model signature now uses SearchGraph |
| rust/routee-compass-powertrain/src/model/ice_energy_model.rs | Traversal model signature now uses SearchGraph |
| rust/routee-compass-powertrain/src/model/charging/simple_charging_model.rs | Updated signatures/tests to SearchGraph |
| rust/routee-compass-powertrain/src/model/bev_energy_model.rs | Traversal model signature now uses SearchGraph |
| rust/routee-compass-core/src/util/geo/polygonal_rtree.rs | Local variable rename (unrelated to search graph) |
| rust/routee-compass-core/src/testing/mock/traversal_model.rs | Mock traversal model updated to SearchGraph |
| rust/routee-compass-core/src/model/traversal/traversal_model.rs | Trait updated: tree param -> graph type/name |
| rust/routee-compass-core/src/model/traversal/default/turn_delays/turn_delay_traversal_model.rs | Uses SearchGraph for prior-edge lookup |
| rust/routee-compass-core/src/model/traversal/default/time/time_traversal_model.rs | Updated traversal hooks to SearchGraph |
| rust/routee-compass-core/src/model/traversal/default/temperature/temperature_traversal_model.rs | Updated traversal hooks to SearchGraph |
| rust/routee-compass-core/src/model/traversal/default/speed/speed_traversal_model.rs | Updated traversal hooks/tests to SearchGraph |
| rust/routee-compass-core/src/model/traversal/default/grade/grade_traversal_model.rs | Updated traversal hooks to SearchGraph |
| rust/routee-compass-core/src/model/traversal/default/elevation/elevation_traversal_model.rs | Updated traversal hooks to SearchGraph |
| rust/routee-compass-core/src/model/traversal/default/distance/distance_traversal_model.rs | Updated traversal hooks to SearchGraph |
| rust/routee-compass-core/src/model/traversal/default/custom/custom_traversal_model.rs | Updated traversal hooks to SearchGraph |
| rust/routee-compass-core/src/model/traversal/default/combined/combined_traversal_model.rs | Propagates SearchGraph through combined model |
| rust/routee-compass-core/src/model/traversal/default/combined/combined_ops.rs | Test types updated to SearchGraph |
| rust/routee-compass-core/src/model/termination/model.rs | Termination checks now accept SearchGraph |
| rust/routee-compass-core/src/model/label/label_model.rs | Docstrings updated to refer to SearchGraph |
| rust/routee-compass-core/src/model/label/label_enum.rs | Docstrings updated to refer to SearchGraph |
| rust/routee-compass-core/src/model/cost/network/network_cost_rate.rs | Cost rate hook signature uses SearchGraph |
| rust/routee-compass-core/src/model/cost/cost_model.rs | Traversal cost computation uses SearchGraph |
| rust/routee-compass-core/src/algorithm/search/search_result.rs | SearchResult stores SearchGraph (field still tree) |
| rust/routee-compass-core/src/algorithm/search/search_pruning.rs | Pruning renamed to prune_graph + new error type |
| rust/routee-compass-core/src/algorithm/search/search_instance.rs | Internal solution container now SearchGraph |
| rust/routee-compass-core/src/algorithm/search/search_graph.rs | New SearchGraph type (renamed from tree) |
| rust/routee-compass-core/src/algorithm/search/search_graph_node.rs | Node type renamed to SearchGraphNode |
| rust/routee-compass-core/src/algorithm/search/search_error.rs | Error variant wraps SearchGraphError |
| rust/routee-compass-core/src/algorithm/search/search_algorithm.rs | Updated comment reference to SearchGraph |
| rust/routee-compass-core/src/algorithm/search/search_algorithm_result.rs | Algorithm result uses Vec<SearchGraph> (field still trees) |
| rust/routee-compass-core/src/algorithm/search/mod.rs | Module exports switched from tree to graph modules |
| rust/routee-compass-core/src/algorithm/search/ksp/svp.rs | Uses SearchGraphNode in KSP logic |
| rust/routee-compass-core/src/algorithm/search/edge_traversal.rs | Edge traversal creation now accepts &SearchGraph |
| rust/routee-compass-core/src/algorithm/search/a_star/frontier_instance.rs | Frontier logic now uses &SearchGraph |
| rust/routee-compass-core/src/algorithm/search/a_star/a_star_algorithm.rs | A* now builds/returns SearchGraph |
| rust/routee-compass-core/src/algorithm/map_matching/model/lcss/lcss_ops.rs | Map matching error variant renamed to graph |
| rust/routee-compass-core/src/algorithm/map_matching/map_matching_error.rs | Error type updated to SearchGraphError |
| rust/routee-compass-codegen/src/generator/traversal.rs | Codegen template imports SearchGraph |
| docs/config.md | Updated wording removing “tree results” phrasing |
Comments suppressed due to low confidence (3)
rust/routee-compass-core/src/algorithm/search/search_graph.rs:15
- The type is documented as a DAG, but
insertdoes not enforce acyclicity (it can create cycles, and backtracking only detects cycles at runtime). Either relax the doc comment to “directed graph” / “intended to be acyclic” or add cycle-prevention checks during insertion.
rust/routee-compass/src/plugin/output/default/traversal/traversal_ops.rs:33 - This output helper is still named
create_tree_geojson(and emits error text “tree GeoJSON”) but now operates on aSearchGraph. To avoid confusion, consider renaming these helpers/messages to*_graph_*(or otherwise clarifying that this is legacy naming for graph output).
rust/routee-compass/src/plugin/output/default/summary/plugin.rs:35 tree_edgesis now computed fromresult.graphs, and the JSON key is stilltree_size_count. If this is truly a graph now, consider renaming the variable/key to avoid confusing downstream consumers; if the key must remain for compatibility, add an explicit comment and at least rename the local variable tograph_edgesfor clarity.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rust/routee-compass-core/src/algorithm/search/search_algorithm_result.rs
Show resolved
Hide resolved
rust/routee-compass-core/src/algorithm/map_matching/map_matching_error.rs
Outdated
Show resolved
Hide resolved
rust/routee-compass/src/plugin/output/default/traversal/traversal_output_format.rs
Outdated
Show resolved
Hide resolved
|
@nreinicke regarding multiedge vs optimal edge: the default behavior is to plot all edges of a search graph. i'm just going to leave it there for this PR and if we want to, we can revisit |
ever since we introduced the LabelModel, search is no longer guaranteed to produce a tree. this PR renames SearchTree to SearchGraph. the description describes it as a directed acyclic graph (DAG). this covers the wider definition of what we build in cases where a node may have multiple parents.
this is mostly to align naming conventions with the upcoming journal paper.