Skip to content
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.

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

Terminate BFSShortestPath#getPath early #1158

Open
Elewyth opened this issue Jul 26, 2023 · 2 comments
Open

Terminate BFSShortestPath#getPath early #1158

Elewyth opened this issue Jul 26, 2023 · 2 comments

Comments

@Elewyth
Copy link

Elewyth commented Jul 26, 2023

 * JGraphT version: 1.5.2
 * Java version (java -version)/platform:  17.0.6

Issue
The current BFSShortestPath implementation of getPath simply calls getPaths and then retrieves the desired path from the SingleSourcePaths. This causes the BFS algorithm to calculate the shortest path to all other (reachable) vertices in the graph, regardless of whether we have already found the shortest path to the desired sink.

For very large graphs this causes a significant overhead, especially if the sink is close to the source. Imagine a graph like:

A -> B -> (huge subgraph)

Using the BFSShortestPath to find the shortest path from A to B would continue to traverse the entire subgraph after B.

Steps to reproduce (small coding example)

var graph = new SimpleDirectedGraph<Integer, DefaultEdge>(DefaultEdge.class);
graph.addVertex(1);
graph.addVertex(2);
graph.addVertex(3);

graph.addEdge(1, 2);
graph.addEdge(2, 3);

var alg = new BFSShortestPath<>(graph);
var result = alg.getPath(1, 2); // also calculates the shortest path from 1 to 3 (1 -> 2 -> 3), even though we already found 2

Expected behaviour
BFSShortestPath#getPath should terminate as soon as the shortest path from source to sink was found.

Other information
E.g. DijkstraShortestPath#getPath has an implementation that terminates early as soon as the shortest path to the sink vertex was found.

@jkinable
Copy link
Collaborator

jkinable commented Aug 1, 2023

That sounds reasonable. Would you be willing to submit a pull request with the suggested code change?

@Pratyushgoyal66
Copy link

Pratyushgoyal66 commented May 19, 2024

Hi @jkinable, I see the issue is still open, can I work on this?

Pratyushgoyal66 added a commit to Pratyushgoyal66/jgrapht that referenced this issue May 19, 2024
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

No branches or pull requests

3 participants