Skip to content

Clarify that dijkstra_search is not a multi-source BFS #1654

Description

@tsondru

Information

  • rustworkx-core version: 0.18.0 (crates.io)
  • Rust version: stable
  • Operating system: Linux

What is happening?

rustworkx_core::traversal::dijkstra_search accepts an iterator of start nodes, but the docs only say "Starting points are the nodes in the iterator starts" and the doc-comment pseudocode describes a single source s. What multiple starts actually do is left unspecified — and the actual behavior is surprising.

Actual behavior (traversal/dijkstra_visit.rs:180-207 at the 0.18.0 tag): starts are processed sequentially, each with a fresh local scores/heap but a shared visited map. Node processing is gated by !visited.visit(node) and edge relaxation by visited.is_visited(&next) — so once an earlier start finalizes a node, later starts can never relax into it, even with a strictly smaller distance. The result is an order-dependent partition ("first start in iteration order to finalize a node wins it"), not the min-distance-from-any-source semantics that BGL's dijkstra visitor and NetworkX's multi_source_dijkstra provide via joint queue seeding.

The same sequential-restart pattern exists in bfs_search/dfs_search, where it is a reasonable "forest" semantics because those events carry no costs. dijkstra_search is different: Discover/EdgeRelaxed events carry costs, so a caller who reads them as shortest distances gets silently order-dependent answers whenever the reachable sets of different starts overlap.

How can we reproduce the issue?

Verified against crates.io rustworkx-core = "0.18.0":

use rustworkx_core::petgraph::graph::{node_index as n, DiGraph};
use rustworkx_core::traversal::{dijkstra_search, DijkstraEvent};

// A -> X (weight 10), B -> X (weight 1)
let mut g: DiGraph<(), u32> = DiGraph::new();
let (a, b, x) = (g.add_node(()), g.add_node(()), g.add_node(()));
g.add_edge(a, x, 10);
g.add_edge(b, x, 1);

let run = |starts: Vec<_>| {
    let mut cost_of_x = None;
    dijkstra_search(&g, starts, |e| Ok::<u32, ()>(*e.weight()), |event| {
        if let DijkstraEvent::Discover(v, cost) = event {
            if v == x { cost_of_x = Some(cost); }
        }
        Ok::<(), ()>(())
    }).unwrap();
    cost_of_x.unwrap()
};

assert_eq!(run(vec![n(0), n(1)]), 10); // starts [A, B]: X discovered at cost 10 (via A — stale)
assert_eq!(run(vec![n(1), n(0)]), 1);  // starts [B, A]: X discovered at cost 1 (the true minimum)

Same graph, same query — the reported cost of X depends purely on start-list order. A true multi-source Dijkstra would report 1 in both cases.

What should happen?

Preferably: seed all starts into a single priority queue at distance zero (the BGL / NetworkX multi_source_dijkstra semantics), so Discover(v, cost) always carries the minimum distance from the nearest start regardless of iteration order. If changing the existing function's behavior is considered too breaking, a separate multi_source_dijkstra_search entry point would serve the same need.

Failing that, at minimum: document the sequential shared-visited semantics explicitly and warn that multi-start is not multi-source shortest path (current workaround: call per-start and take the per-node min).

Happy to help with a PR for whichever direction the maintainers prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions