Conversation
📝 WalkthroughWalkthroughA Jupyter notebook demonstrating the Bellman-Ford shortest path algorithm is added, showing implementations using both SciPy and NetworkX libraries with example weighted directed graphs, algorithm execution, and formatted output of distances and predecessors. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
belmann_ford/DEMO.ipynb (2)
45-50: Consider adding negative cycle detection for completeness.The standard Bellman-Ford algorithm includes a final pass after V-1 iterations to detect negative cycles. Since this is a demo notebook, adding this check would make the implementation more complete and educational.
💡 Optional: Add negative cycle detection
for _ in range(4): # v-1 iterations for u, v, w in edges: # 'w' is the 'new' weight if distances[u] < float('inf'): distances[v] = min(distances[v], distances[u] + w) +# check for negative cycles (one more iteration) +has_negative_cycle = False +for u, v, w in edges: + if distances[u] < float('inf') and distances[u] + w < distances[v]: + has_negative_cycle = True + break + +if has_negative_cycle: + print("Graph contains a negative cycle!") +else: + print("Shortest distances from vertex A (0):", distances) -print("Shortest distances from vertex A (0):", distances)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@belmann_ford/DEMO.ipynb` around lines 45 - 50, Add a final relaxation pass after the existing V-1 iterations (the "for _ in range(4):" loop) to detect negative-weight cycles: iterate over edges (the same "for u, v, w in edges:" loop) and if any distances[u] < float('inf') and distances[u] + w < distances[v] then report or raise that a negative cycle exists (e.g., print a message or set a flag) instead of only printing distances; update the demo to clearly indicate when a negative cycle is detected versus when shortest distances are valid.
88-92: Consider using NetworkX's built-in Bellman-Ford.NetworkX provides
nx.single_source_bellman_fordwhich handles everything directly without needing to convert to a numpy array and use scipy. This would simplify the code if the goal is to demonstrate shortest path computation rather than scipy specifically.💡 Alternative using NetworkX directly
# Simpler alternative using NetworkX directly distances, paths = nx.single_source_bellman_ford(G, source='A') print("distances:", distances) print("paths:", paths)Note: The current scipy approach is valid and demonstrates interoperability between NetworkX and scipy, which has educational value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@belmann_ford/DEMO.ipynb` around lines 88 - 92, The notebook currently converts the NetworkX graph G to a NumPy weight matrix W and calls scipy's bellman_ford (bellman_ford(W, indices=0, return_predecessors=True)); replace that with NetworkX's built-in call to simplify and avoid conversion by using nx.single_source_bellman_ford(G, source=...) to obtain distances and paths directly, updating any subsequent uses of the returned variables (distances, predecessors/paths) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@belmann_ford/DEMO.ipynb`:
- Around line 1-2: The repository uses the misspelled directory name
"belmann_ford" (and file DEMO.ipynb) but the algorithm is Bellman–Ford; rename
the directory to "bellman_ford" and update all references to it (imports,
module/package names, relative paths, notebook links, README/documentation,
tests, and CI scripts) so nothing breaks. Search for the symbol "belmann_ford"
and replace with "bellman_ford", update any Python imports or module paths that
reference that package, and ensure the notebook filename DEMO.ipynb and any
references to it are moved/updated accordingly; run tests and CI to verify no
leftover references remain.
---
Nitpick comments:
In `@belmann_ford/DEMO.ipynb`:
- Around line 45-50: Add a final relaxation pass after the existing V-1
iterations (the "for _ in range(4):" loop) to detect negative-weight cycles:
iterate over edges (the same "for u, v, w in edges:" loop) and if any
distances[u] < float('inf') and distances[u] + w < distances[v] then report or
raise that a negative cycle exists (e.g., print a message or set a flag) instead
of only printing distances; update the demo to clearly indicate when a negative
cycle is detected versus when shortest distances are valid.
- Around line 88-92: The notebook currently converts the NetworkX graph G to a
NumPy weight matrix W and calls scipy's bellman_ford (bellman_ford(W, indices=0,
return_predecessors=True)); replace that with NetworkX's built-in call to
simplify and avoid conversion by using nx.single_source_bellman_ford(G,
source=...) to obtain distances and paths directly, updating any subsequent uses
of the returned variables (distances, predecessors/paths) accordingly.
Comment on lines
+1
to
+2
| { | ||
| "cells": [ |
There was a problem hiding this comment.
Typo in directory name: "belmann" → "bellman"
The directory is named belmann_ford but the algorithm is named after Richard Bellman (single 'n' after 'bell'): bellman_ford.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@belmann_ford/DEMO.ipynb` around lines 1 - 2, The repository uses the
misspelled directory name "belmann_ford" (and file DEMO.ipynb) but the algorithm
is Bellman–Ford; rename the directory to "bellman_ford" and update all
references to it (imports, module/package names, relative paths, notebook links,
README/documentation, tests, and CI scripts) so nothing breaks. Search for the
symbol "belmann_ford" and replace with "bellman_ford", update any Python imports
or module paths that reference that package, and ensure the notebook filename
DEMO.ipynb and any references to it are moved/updated accordingly; run tests and
CI to verify no leftover references remain.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
bellman_ford/DEMO.ipynb (2)
42-50: Make the manual example detect negative cycles too.This cell stops after
V - 1relaxations and prints whatever distances it has. If someone tweaksedgesto include a reachable negative cycle, this cell will still emit numbers while the SciPy example below raisesNegativeCycleError, teaching two different behaviors for the same algorithm.Suggested change
-# assuming A-E are indexed as 0-4 -distances = [float('inf')] * 5 +num_vertices = 1 + max(max(u, v) for u, v, _ in edges) +distances = [float('inf')] * num_vertices distances[0] = 0 -for _ in range(4): # v-1 iterations +for _ in range(num_vertices - 1): for u, v, w in edges: # 'w' is the 'new' weight if distances[u] < float('inf'): distances[v] = min(distances[v], distances[u] + w) + +for u, v, w in edges: + if distances[u] < float('inf') and distances[u] + w < distances[v]: + raise ValueError("Graph contains a reachable negative-weight cycle") print("Shortest distances from vertex A (0):", distances)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bellman_ford/DEMO.ipynb` around lines 42 - 50, The manual Bellman-Ford example stops after v-1 relaxations and doesn't detect negative cycles; after the existing loop that runs "for _ in range(4):" (i.e., V-1 iterations) add one additional pass over edges (the u, v, w iteration) to check if any distances[v] can still be decreased (and only consider edges where distances[u] is finite), and if so raise or print a NegativeCycleError consistent with the SciPy example; reference the variables distances and edges and perform this extra check rather than printing results immediately so reachable negative cycles are detected and reported.
88-92: Use sparse matrix conversion to keep zero-weight edges representable.
nx.to_numpy_array()defaultsnonedge=0.0, and SciPy's dense csgraph routines treat0as "no edge". This means any zero-weight edges in the graph will become indistinguishable from missing edges, even though Bellman-Ford should handle them. Sparse format avoids this ambiguity and also lets you control node ordering explicitly.Suggested change
-# convert to an adjacency/weight matrix -W = nx.to_numpy_array(G, weight='weight') +nodes = list(G.nodes()) +# keep zero-weight edges representable and make node ordering explicit +W = nx.to_scipy_sparse_array(G, nodelist=nodes, weight='weight', format='csr') -# run scipy's bellman_ford on the weight matrix from source index 0 +# run scipy's bellman_ford from source index 0 distances, predecessors = bellman_ford(W, indices=0, return_predecessors=True) - -nodes = list(G.nodes()) print("nodes:", nodes)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bellman_ford/DEMO.ipynb` around lines 88 - 92, The current conversion uses nx.to_numpy_array(G, weight='weight') which maps nonedges to 0.0 and makes zero-weight edges indistinguishable from missing edges for SciPy's csgraph routines; instead convert the NetworkX graph G to a SciPy sparse adjacency/weight matrix (e.g., via nx.to_scipy_sparse_array or nx.to_scipy_sparse_matrix with weight='weight', format='csr' and an explicit nodelist if deterministic ordering is required) and pass that sparse matrix to bellman_ford (keeping indices=0 and return_predecessors=True) so zero-weight edges are preserved and node ordering is controlled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@bellman_ford/DEMO.ipynb`:
- Around line 42-50: The manual Bellman-Ford example stops after v-1 relaxations
and doesn't detect negative cycles; after the existing loop that runs "for _ in
range(4):" (i.e., V-1 iterations) add one additional pass over edges (the u, v,
w iteration) to check if any distances[v] can still be decreased (and only
consider edges where distances[u] is finite), and if so raise or print a
NegativeCycleError consistent with the SciPy example; reference the variables
distances and edges and perform this extra check rather than printing results
immediately so reachable negative cycles are detected and reported.
- Around line 88-92: The current conversion uses nx.to_numpy_array(G,
weight='weight') which maps nonedges to 0.0 and makes zero-weight edges
indistinguishable from missing edges for SciPy's csgraph routines; instead
convert the NetworkX graph G to a SciPy sparse adjacency/weight matrix (e.g.,
via nx.to_scipy_sparse_array or nx.to_scipy_sparse_matrix with weight='weight',
format='csr' and an explicit nodelist if deterministic ordering is required) and
pass that sparse matrix to bellman_ford (keeping indices=0 and
return_predecessors=True) so zero-weight edges are preserved and node ordering
is controlled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit