-
Notifications
You must be signed in to change notification settings - Fork 54
Check for GPU for notebook run #87
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
Conversation
WalkthroughAdded a reusable notebook GPU-detection helper Changes
Sequence Diagram(s)sequenceDiagram
participant NB as Notebook Cell
participant CG as check_gpu()
participant SP as subprocess.run
participant DP as IPython.display
NB->>CG: invoke check_gpu()
CG->>SP: run(["nvidia-smi"], capture_output=True, text=True, timeout=...)
alt nvidia-smi succeeds
SP-->>CG: stdout (GPU info)
CG->>DP: display(HTML(green success block + escaped snippet))
else nvidia-smi fails / exception
SP-->>CG: raises/returns error
CG->>DP: display(HTML(red failure block + Colab/Docker guidance))
end
DP-->>NB: rendered HTML banner
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
♻️ Duplicate comments (11)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb (1)
39-79
: Duplicate security risk and refactoring opportunity.This code has the same security vulnerability as in
cvrp_daily_deliveries.ipynb
:def check_gpu(): try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() + output = subprocess.check_output(["nvidia-smi"], shell=False).decode()The identical implementation across multiple notebooks (20+ according to the PR summary) is a maintenance burden. Consider:
- Creating a shared utility module (e.g.,
utils/gpu_check.py
)- Importing and calling the function from notebooks
- This would allow fixing bugs in one place and ensure consistency
Example structure:
# In utils/gpu_check.py def check_gpu(): # Implementation here pass # In notebooks from utils.gpu_check import check_gpu check_gpu()intra-factory_transport/intra-factory_transport.ipynb (1)
55-95
: Address code duplication and potential IndexError.This GPU check function is identical to the one in other notebooks. See the review comment on
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
for the same concerns regarding:
- Code duplication across notebooks
output.splitlines()[2]
IndexError risk- Broad exception handling
sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb (1)
49-89
: Address code duplication and potential IndexError.This GPU check function is identical to the one in other notebooks. See the review comment on
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
for the same concerns.sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb (1)
50-90
: Address code duplication and potential IndexError.This GPU check function is identical to the one in other notebooks. See the review comment on
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
for the same concerns.diet_optimization/diet_optimization_milp.ipynb (1)
36-76
: Address code duplication and potential IndexError.This GPU check function is identical to the one in other notebooks. See the review comment on
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
for the same concerns.routing_optimization_over_server/cvrptw_service_team_routing.ipynb (1)
63-103
: Address code duplication and potential IndexError.This GPU check function is identical to the one in other notebooks. See the review comment on
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
for the same concerns.PuLP_integration_example/Simple_MIP_pulp.ipynb (1)
36-76
: Address code duplication and potential IndexError.This GPU check function is identical to the one in other notebooks. See the review comment on
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
for the same concerns.PuLP_integration_example/Sudoku_pulp.ipynb (1)
38-78
: Address code duplication and potential IndexError.This GPU check function is identical to the one in other notebooks. See the review comment on
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
for the same concerns.PuLP_integration_example/Production_Planning_Example_Pulp.ipynb (3)
147-147
: Consider replacing star import with explicit imports.Star imports reduce clarity. Consider explicitly importing the required PuLP symbols.
36-76
: Security and reliability concerns in GPU detection.This function has the same issues as in
Simple_LP_pulp.ipynb
:
- Security risk: Using
shell=True
withsubprocess.check_output()
creates a shell injection vulnerability.- Fragile parsing: Line 45 accesses
splitlines()[2]
without bounds checking.- Broad exception handling: Line 48 catches bare
Exception
instead of specific exceptions.Apply the same fix as suggested for
Simple_LP_pulp.ipynb
:-def check_gpu(): - try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() +def check_gpu(): + try: + output = subprocess.check_output(["nvidia-smi"], shell=False).decode() + lines = output.splitlines() + gpu_line = lines[2] if len(lines) > 2 else "GPU info unavailable" display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> - <pre>{output.splitlines()[2]}</pre> + <pre>{gpu_line}</pre> </div> """)) - except Exception: + except (subprocess.CalledProcessError, FileNotFoundError):
36-76
: Extract duplicated GPU check function to a shared module.This is the same
check_gpu()
function duplicated fromSimple_LP_pulp.ipynb
. Extract it to a shared utility module to eliminate duplication.
🧹 Nitpick comments (6)
diet_optimization/diet_optimization_lp.ipynb (1)
221-221
: Optional: Simplify redundant f-string.The f-string wrapper is unnecessary here since there's only a single variable interpolation.
Apply this diff:
- var = problem.addVariable(name=f"{food_name}", vtype=VType.CONTINUOUS, lb=0.0, ub=float('inf')) + var = problem.addVariable(name=food_name, vtype=VType.CONTINUOUS, lb=0.0, ub=float('inf'))GAMSPy_integration_example/trnsport_cuopt.ipynb (1)
61-103
: Reduce duplication across notebooksThe same check_gpu() appears in many notebooks. Consider a small shared helper (e.g., utils/checks.py) and import it to keep one source of truth.
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb (1)
115-116
: Avoid star import; import only needed symbols
from ...solver_parameters import *
obscures names and triggers lint issues. Import explicit constants used.-from cuopt.linear_programming.solver.solver_parameters import * +from cuopt.linear_programming.solver.solver_parameters import ( + CUOPT_PDLP_SOLVER_MODE, + CUOPT_TIME_LIMIT, + CUOPT_LOG_TO_CONSOLE, +)Also applies to: 129-133
PuLP_integration_example/Simple_LP_pulp.ipynb (1)
124-124
: Consider replacing star import with explicit imports.While star imports are more commonly accepted in notebooks, explicitly importing the required symbols (
LpProblem
,LpMinimize
,LpVariable
,lpSum
,CUOPT
,value
) improves clarity and helps catch typos early.workforce_optimization/workforce_optimization_milp.ipynb (2)
349-357
: Use binary variable type instead of INTEGER + [0,1] bounds (if supported)Cleaner and potentially faster for MIP: declare binary vars directly.
Apply this if cuOpt supports it:
- var = problem.addVariable(name=var_name, vtype=VType.INTEGER, lb=0.0, ub=1.0) + var = problem.addVariable(name=var_name, vtype=VType.BINARY)Please confirm VType.BINARY exists in your cuOpt version.
579-581
: Remove unnecessary f-stringNo placeholders present; simplify to avoid linter F541.
- print(f"\nOptimal Solution Found!") + print("\nOptimal Solution Found!")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
GAMSPy_integration_example/trnsport_cuopt.ipynb
(1 hunks)PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_LP_pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_MIP_pulp.ipynb
(1 hunks)PuLP_integration_example/Sudoku_pulp.ipynb
(1 hunks)diet_optimization/diet_optimization_lp.ipynb
(1 hunks)diet_optimization/diet_optimization_milp.ipynb
(1 hunks)intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
(1 hunks)intra-factory_transport/intra-factory_transport.ipynb
(1 hunks)last_mile_delivery/cvrp_daily_deliveries.ipynb
(1 hunks)last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
(1 hunks)last_mile_delivery/cvrptw_service_team_routing.ipynb
(1 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
(1 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
(1 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
(1 hunks)routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
(1 hunks)routing_optimization_over_server/cvrptw_service_team_routing.ipynb
(1 hunks)sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
(1 hunks)sample_lp_sever_notebooks/linear-programming.ipynb
(1 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
(1 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
(1 hunks)workforce_optimization/workforce_optimization_milp.ipynb
(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.0)
sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
51-51: Shebang should contain python
, pytest
, or uv run
(EXE003)
51-51: Shebang is present but file is not executable
(EXE001)
51-51: Shebang should be at the beginning of the file
(EXE005)
54-54: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
65-65: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
routing_optimization_over_server/cvrptw_service_team_routing.ipynb
66-66: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
69-69: Redefinition of unused os
from line 58
Remove definition: os
(F811)
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
51-51: Shebang should contain python
, pytest
, or uv run
(EXE003)
51-51: Shebang is present but file is not executable
(EXE001)
51-51: Shebang should be at the beginning of the file
(EXE005)
54-54: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
51-51: Shebang should contain python
, pytest
, or uv run
(EXE003)
51-51: Shebang is present but file is not executable
(EXE001)
51-51: Shebang should be at the beginning of the file
(EXE005)
54-54: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
PuLP_integration_example/Sudoku_pulp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
63-63: from pulp import *
used; unable to detect undefined names
(F403)
70-70: Ambiguous variable name: l
(E741)
76-76: LpProblem
may be undefined, or defined from star imports
(F405)
79-79: LpVariable
may be undefined, or defined from star imports
(F405)
86-86: lpSum
may be undefined, or defined from star imports
(F405)
91-91: lpSum
may be undefined, or defined from star imports
(F405)
94-94: lpSum
may be undefined, or defined from star imports
(F405)
97-97: lpSum
may be undefined, or defined from star imports
(F405)
140-140: CUOPT
may be undefined, or defined from star imports
(F405)
143-143: LpStatus
may be undefined, or defined from star imports
(F405)
152-152: value
may be undefined, or defined from star imports
(F405)
sample_lp_sever_notebooks/linear-programming.ipynb
51-51: Shebang should contain python
, pytest
, or uv run
(EXE003)
51-51: Shebang is present but file is not executable
(EXE001)
51-51: Shebang should be at the beginning of the file
(EXE005)
54-54: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
78-78: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
81-81: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
diet_optimization/diet_optimization_lp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
206-206: f-string without any placeholders
Remove extraneous f
prefix
(F541)
PuLP_integration_example/Simple_LP_pulp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
49-49: Shebang should contain python
, pytest
, or uv run
(EXE003)
49-49: Shebang is present but file is not executable
(EXE001)
49-49: Shebang should be at the beginning of the file
(EXE005)
53-53: from pulp import *
used; unable to detect undefined names
(F403)
69-69: LpProblem
may be undefined, or defined from star imports
(F405)
69-69: LpMinimize
may be undefined, or defined from star imports
(F405)
70-70: LpVariable
may be undefined, or defined from star imports
(F405)
71-71: lpSum
may be undefined, or defined from star imports
(F405)
74-74: lpSum
may be undefined, or defined from star imports
(F405)
77-77: CUOPT
may be undefined, or defined from star imports
(F405)
80-80: value
may be undefined, or defined from star imports
(F405)
PuLP_integration_example/Simple_MIP_pulp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
55-55: from pulp import *
used; unable to detect undefined names
(F403)
58-58: LpProblem
may be undefined, or defined from star imports
(F405)
58-58: LpMinimize
may be undefined, or defined from star imports
(F405)
61-61: LpVariable
may be undefined, or defined from star imports
(F405)
62-62: LpVariable
may be undefined, or defined from star imports
(F405)
75-75: CUOPT
may be undefined, or defined from star imports
(F405)
78-78: LpStatus
may be undefined, or defined from star imports
(F405)
79-79: value
may be undefined, or defined from star imports
(F405)
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
61-61: Found useless expression. Either assign it to a variable or remove it.
(B018)
GAMSPy_integration_example/trnsport_cuopt.ipynb
63-63: Found useless expression. Either assign it to a variable or remove it.
(B018)
70-70: Found useless expression. Either assign it to a variable or remove it.
(B018)
85-85: Found useless expression. Either assign it to a variable or remove it.
(B018)
diet_optimization/diet_optimization_milp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
208-208: f-string without any placeholders
Remove extraneous f
prefix
(F541)
PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
49-49: Shebang should contain python
, pytest
, or uv run
(EXE003)
49-49: Shebang is present but file is not executable
(EXE001)
49-49: Shebang should be at the beginning of the file
(EXE005)
52-52: from pulp import *
used; unable to detect undefined names
(F403)
55-55: LpProblem
may be undefined, or defined from star imports
(F405)
55-55: LpMaximize
may be undefined, or defined from star imports
(F405)
58-58: LpVariable
may be undefined, or defined from star imports
(F405)
59-59: LpVariable
may be undefined, or defined from star imports
(F405)
60-60: LpVariable
may be undefined, or defined from star imports
(F405)
76-76: CUOPT
may be undefined, or defined from star imports
(F405)
79-79: LpStatus
may be undefined, or defined from star imports
(F405)
83-83: value
may be undefined, or defined from star imports
(F405)
routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
51-51: Shebang should contain python
, pytest
, or uv run
(EXE003)
51-51: Shebang is present but file is not executable
(EXE001)
51-51: Shebang should be at the beginning of the file
(EXE005)
65-65: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
68-68: Redefinition of unused os
from line 58
Remove definition: os
(F811)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
67-67: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
67-67: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
74-74: CUOPT_PDLP_SOLVER_MODE
may be undefined, or defined from star imports
(F405)
76-76: CUOPT_TIME_LIMIT
may be undefined, or defined from star imports
(F405)
77-77: CUOPT_LOG_TO_CONSOLE
may be undefined, or defined from star imports
(F405)
workforce_optimization/workforce_optimization_milp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
138-138: Loop control variable shift
not used within loop body
(B007)
155-155: Loop control variable worker
not used within loop body
(B007)
202-202: f-string without any placeholders
Remove extraneous f
prefix
(F541)
274-274: Loop control variable shift
not used within loop body
(B007)
🔇 Additional comments (5)
diet_optimization/diet_optimization_lp.ipynb (1)
104-432
: Well-structured diet optimization implementation.The diet optimization problem is correctly formulated and solved:
- Decision variables properly defined with non-negative continuous bounds
- Objective function minimizes total food cost
- Nutrition constraints correctly implement min/max bounds using separate constraints when needed
- Solution printing includes tolerance handling (
1e-6
) for floating-point precision- Additional constraint demo effectively shows model extensibility
The logic is sound and follows optimization modeling best practices.
PuLP_integration_example/Simple_LP_pulp.ipynb (2)
8-28
: LGTM! Clear notebook introduction and setup instructions.The introductory markdown provides clear context about cuOpt, PuLP integration, and runtime requirements for both Colab and Docker environments.
88-88
: LGTM! Pinned PuLP version ensures reproducibility.Pinning
pulp==3.2.0
provides a stable, reproducible dependency.PuLP_integration_example/Production_Planning_Example_Pulp.ipynb (2)
109-137
: LGTM! Comprehensive problem description.The markdown clearly explains the production planning MIP problem with profit calculations, resource constraints, and the conditional machine logic. The key features breakdown is particularly helpful.
147-166
: LGTM! Problem formulation matches the description.The PuLP problem setup correctly implements the constraints and objective function described in the markdown, including the mixed integer and binary variables.
routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb (1)
39-88
: Harden the GPU check helper
check_gpu()
currently shells out viasubprocess.check_output(..., shell=True)
and swallows every exception. That is unsafe and brittle (it is easy to accidentally execute a shell metacharacter, and we can’t tell a genuine failure apart from other errors). It also assumesnvidia-smi
prints at least three lines, which raises when that is not true. Please move to a guardedsubprocess.run([...], check=True, capture_output=True, timeout=3)
call, dropshell=True
, check the collected output defensively, and only catch the expectedsubprocess
failures. Including the exception text in the fallback HTML keeps the UX intact while avoiding false positives.-import subprocess -from IPython.display import display, HTML - -def check_gpu(): - try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() - display(HTML(f""" - <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> - <h3>✅ GPU is enabled</h3> - <pre>{output.splitlines()[2]}</pre> - </div> - """)) - except Exception: - display(HTML(""" - <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> - <h3>⚠️ GPU not detected!</h3> - <p>This notebook requires a <b>GPU runtime</b>.</p> - <ol> - <li>Click on <b>Runtime → Change runtime type</b></li> - <li>Set <b>Hardware accelerator</b> to <b>GPU</b></li> - <li>Then click <b>Save</b> and <b>Runtime → Restart runtime</b>.</li> - </ol> - </div> - """)) +import subprocess +from IPython.display import display, HTML + +def check_gpu(): + try: + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=3, + ) + lines = result.stdout.splitlines() + summary = lines[0] if lines else "nvidia-smi ran but returned no GPU lines." + display( + HTML( + f""" + <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> + <h3>✅ GPU is enabled</h3> + <pre>{summary}</pre> + </div> + """ + ) + ) + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display( + HTML( + f""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU not detected!</h3> + <p>This notebook requires a <b>GPU runtime</b>.</p> + <ol> + <li>Click on <b>Runtime → Change runtime type</b></li> + <li>Set <b>Hardware accelerator</b> to <b>GPU</b></li> + <li>Then click <b>Save</b> and <b>Runtime → Restart runtime</b>.</li> + </ol> + <p><b>Details:</b> {exc}</p> + </div> + """ + ) + )portfolio_optimization/cvar_portfolio_optimization.ipynb (1)
857-857
: Fix typo in success message.There's a typo: "successfuli!" should be "successful!"
- print(f"\nOptimization successfuli!") + print(f"\nOptimization successful!")
♻️ Duplicate comments (6)
last_mile_delivery/cvrp_daily_deliveries.ipynb (1)
67-92
: All concerns have been previously raised.The issues with this function (security risk from
shell=True
, fragile parsing, broad exception handling, and code duplication across 20+ notebooks) have been comprehensively addressed in previous review comments.last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb (1)
45-67
: Hardencheck_gpu()
command invocation and HTML handling.Running
subprocess.check_output("nvidia-smi", shell=True)
is brittle (TDZ foroutput.splitlines()[2]
), keeps the prior shell=True injection surface, and emits raw GPU text into HTML without escaping. Please switch to["nvidia-smi"]
(no shell), guard the parsed line, HTML-escape it, and return a truthy value so later cells can gate on GPU availability. For example:-import subprocess -from IPython.display import display, HTML +import html +import subprocess +from IPython.display import display, HTML @@ -def check_gpu(): - try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() - display(HTML(f""" - <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> - <h3>✅ GPU is enabled</h3> - <pre>{output.splitlines()[2]}</pre> - </div> - """)) - except Exception: +def check_gpu(): + try: + lines = subprocess.check_output(["nvidia-smi"], text=True).splitlines() + summary = next((line for line in lines if line.strip()), lines[0] if lines else "nvidia-smi OK") + display(HTML(f""" + <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> + <h3>✅ GPU is enabled</h3> + <pre>{html.escape(summary)}</pre> + </div> + """)) + return True + except Exception: display(HTML(""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3> @@ - """)) - -check_gpu() + """)) + return False + +_ = check_gpu()routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb (1)
42-67
: Hardencheck_gpu()
The helper here has the same
shell=True
+ blanketexcept
+ brittle indexing problems as the other notebooks. Please apply the safersubprocess.run([...], check=True, capture_output=True, timeout=3)
pattern and only catchCalledProcessError
,FileNotFoundError
, andTimeoutExpired
, while handling short outputs defensively.-import subprocess -from IPython.display import display, HTML - -def check_gpu(): - try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() - display(HTML(f""" - <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> - <h3>✅ GPU is enabled</h3> - <pre>{output.splitlines()[2]}</pre> - </div> - """)) - except Exception: - display(HTML(""" - <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> - <h3>⚠️ GPU not detected!</h3> - <p>This notebook requires a <b>GPU runtime</b>.</p> - <ol> - <li>Click on <b>Runtime → Change runtime type</b></li> - <li>Set <b>Hardware accelerator</b> to <b>GPU</b></li> - <li>Then click <b>Save</b> and <b>Runtime → Restart runtime</b>.</li> - </ol> - </div> - """)) +import subprocess +from IPython.display import display, HTML + +def check_gpu(): + try: + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=3, + ) + lines = result.stdout.splitlines() + summary = lines[0] if lines else "nvidia-smi ran but returned no GPU lines." + display( + HTML( + f""" + <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> + <h3>✅ GPU is enabled</h3> + <pre>{summary}</pre> + </div> + """ + ) + ) + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display( + HTML( + f""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU not detected!</h3> + <p>This notebook requires a <b>GPU runtime</b>.</p> + <ol> + <li>Click on <b>Runtime → Change runtime type</b></li> + <li>Set <b>Hardware accelerator</b> to <b>GPU</b></li> + <li>Then click <b>Save</b> and <b>Runtime → Restart runtime</b>.</li> + </ol> + <p><b>Details:</b> {exc}</p> + </div> + """ + ) + )PuLP_integration_example/Simple_LP_pulp.ipynb (1)
37-62
: Harden GPU detection against command injection and parsing errors.
shell=True
keeps the command-injection risk in a cell that may be run by others, the code still indexessplitlines()[2]
without verifying the length, and the bareexcept Exception
hides real failure causes. Please switch to an absolute binary path withshell=False
, add line‑count guards, and catchFileNotFoundError
/subprocess.CalledProcessError
explicitly (optionally surfacing the captured stderr) so the UI stays informative without masking bugs.-import subprocess -from IPython.display import display, HTML +import shutil +import subprocess +from IPython.display import display, HTML @@ def check_gpu(): try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() - display(HTML(f""" + nvidia_smi = shutil.which("nvidia-smi") + if not nvidia_smi: + raise FileNotFoundError("nvidia-smi binary not found") + output = subprocess.check_output([nvidia_smi], stderr=subprocess.STDOUT).decode() + lines = output.splitlines() + gpu_line = lines[2] if len(lines) > 2 else (lines[0] if lines else "GPU detected") + display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> - <pre>{output.splitlines()[2]}</pre> + <pre>{gpu_line}</pre> </div> """)) - except Exception: + except FileNotFoundError: + display(HTML(""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU tooling not found!</h3> + <p>The <code>nvidia-smi</code> binary is missing; install NVIDIA drivers or run inside a GPU-enabled environment.</p> + </div> + """)) + except subprocess.CalledProcessError as err: + details = err.output.decode(errors="ignore") if err.output else str(err) + display(HTML(f""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU query failed!</h3> + <p><code>nvidia-smi</code> returned a non-zero status.</p> + <pre>{details}</pre> + </div> + """))diet_optimization/diet_optimization_lp.ipynb (1)
36-77
: Secure and harden the GPU check helper.This cell still launches
nvidia-smi
via the shell, blindly indexes line 2, and swallows every exception. Please resolve the earlier feedback by switching toshell=False
with the resolved binary path, validating the output length, and catching specific exceptions so we don’t hide real configuration problems.-import subprocess -from IPython.display import display, HTML +import shutil +import subprocess +from IPython.display import display, HTML @@ def check_gpu(): try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() + nvidia_smi = shutil.which("nvidia-smi") + if not nvidia_smi: + raise FileNotFoundError("nvidia-smi binary not found") + output = subprocess.check_output([nvidia_smi], stderr=subprocess.STDOUT).decode() + lines = output.splitlines() + gpu_info = lines[2] if len(lines) > 2 else (lines[0] if lines else "GPU detected") display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> - <pre>{output.splitlines()[2]}</pre> + <pre>{gpu_info}</pre> </div> """)) - except Exception: + except FileNotFoundError: + display(HTML(""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU tooling not found!</h3> + <p>The <code>nvidia-smi</code> binary is missing; install NVIDIA drivers or enable GPU support for this runtime.</p> + </div> + """)) + except subprocess.CalledProcessError as err: + detail = err.output.decode(errors="ignore") if err.output else str(err) + display(HTML(f""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU query failed!</h3> + <p><code>nvidia-smi</code> exited with a non-zero status.</p> + <pre>{detail}</pre> + </div> + """))PuLP_integration_example/Production_Planning_Example_Pulp.ipynb (1)
36-62
: Resolve the remaining security and robustness gaps incheck_gpu()
.Running
nvidia-smi
withshell=True
leaves a command-injection path, indexingsplitlines()[2]
can still throw, and the blanketexcept Exception
hides actionable diagnostics. Please adopt the hardened pattern (explicit binary resolution,shell=False
, guarded parsing, targeted exception handling) so this cell is safe to run in shared environments.-import subprocess -from IPython.display import display, HTML +import shutil +import subprocess +from IPython.display import display, HTML @@ def check_gpu(): try: - output = subprocess.check_output("nvidia-smi", shell=True).decode() + nvidia_smi = shutil.which("nvidia-smi") + if not nvidia_smi: + raise FileNotFoundError("nvidia-smi binary not found") + output = subprocess.check_output([nvidia_smi], stderr=subprocess.STDOUT).decode() + lines = output.splitlines() + gpu_line = lines[2] if len(lines) > 2 else (lines[0] if lines else "GPU detected") display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> - <pre>{output.splitlines()[2]}</pre> + <pre>{gpu_line}</pre> </div> """)) - except Exception: + except FileNotFoundError: + display(HTML(""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU tooling not found!</h3> + <p>Install NVIDIA drivers or switch to a GPU-backed runtime so <code>nvidia-smi</code> is available.</p> + </div> + """)) + except subprocess.CalledProcessError as err: + details = err.output.decode(errors="ignore") if err.output else str(err) + display(HTML(f""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU query failed!</h3> + <p><code>nvidia-smi</code> returned a non-zero status.</p> + <pre>{details}</pre> + </div> + """))
🧹 Nitpick comments (4)
last_mile_delivery/cvrp_daily_deliveries.ipynb (1)
115-116
: Clarify CUDA version selection in installation commands.Add explanatory comments above each pip line so users know which to enable:
# Install cuOpt # For CUDA 12.x environments: -#!pip install --upgrade --extra-index-url https://pypi.nvidia.com cuopt-cu12 nvidia-nvjitlink-cu12 rapids-logger==0.1.19 +#!pip install --upgrade --extra-index-url https://pypi.nvidia.com cuopt-cu12 nvidia-nvjitlink-cu12 rapids-logger==0.1.19 # For CUDA 13.x environments: -#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu13 nvidia-nvjitlink-cu13 rapids-logger==0.1.19 +#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu13 nvidia-nvjitlink-cu13 rapids-logger==0.1.19portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb (1)
985-1018
: Drop the redundant f-string prefix
print(f"\nConcentration Analysis:")
is an f-string without interpolations, which triggers the Ruff warning and adds noise. It can just be a regular string literal.- print(f"\nConcentration Analysis:") + print("\nConcentration Analysis:")portfolio_optimization/cvar_portfolio_optimization.ipynb (2)
105-113
: Consider more specific exception handling.The GPU check function uses a bare
except Exception:
which catches all exceptions. While this ensures the notebook continues to run, it would be more robust to catch specific exceptions likesubprocess.CalledProcessError
orFileNotFoundError
. This would help distinguish between different failure scenarios (e.g., nvidia-smi not found vs. no GPU available).Consider this refinement:
def check_gpu(): try: output = subprocess.check_output("nvidia-smi", shell=True).decode() display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> <pre>{output.splitlines()[2]}</pre> </div> """)) - except Exception: + except (subprocess.CalledProcessError, FileNotFoundError, OSError): display(HTML(""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3>
117-122
: Instructions assume Google Colab environment.The error message provides instructions specific to Google Colab ("Runtime → Change runtime type"), but users might run this notebook in other environments (JupyterLab, VS Code, etc.). Consider making the message more generic or detecting the environment.
You could add a note like: "If running in Google Colab: Click on Runtime → Change runtime type..." or detect the environment and provide appropriate instructions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
GAMSPy_integration_example/trnsport_cuopt.ipynb
(3 hunks)PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
(5 hunks)PuLP_integration_example/Simple_LP_pulp.ipynb
(4 hunks)PuLP_integration_example/Simple_MIP_pulp.ipynb
(2 hunks)PuLP_integration_example/Sudoku_pulp.ipynb
(2 hunks)diet_optimization/diet_optimization_lp.ipynb
(1 hunks)diet_optimization/diet_optimization_milp.ipynb
(1 hunks)intra-factory_transport/intra-factory_transport.ipynb
(3 hunks)last_mile_delivery/cvrp_daily_deliveries.ipynb
(3 hunks)last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
(3 hunks)last_mile_delivery/cvrptw_service_team_routing.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
(2 hunks)portfolio_optimization/cvar_portfolio_optimization.ipynb
(3 hunks)routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
(2 hunks)routing_optimization_over_server/cvrptw_service_team_routing.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
(2 hunks)workforce_optimization/workforce_optimization_milp.ipynb
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- sample_lp_sever_notebooks/linear-programming.ipynb
- last_mile_delivery/cvrptw_service_team_routing.ipynb
- workforce_optimization/workforce_optimization_milp.ipynb
- intra-factory_transport/intra-factory_transport.ipynb
- sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
🧰 Additional context used
🪛 Ruff (0.14.0)
diet_optimization/diet_optimization_milp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
208-208: f-string without any placeholders
Remove extraneous f
prefix
(F541)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
38-38: Shebang should contain python
, pytest
, or uv run
(EXE003)
38-38: Shebang is present but file is not executable
(EXE001)
38-38: Shebang should be at the beginning of the file
(EXE005)
39-39: Shebang should contain python
, pytest
, or uv run
(EXE003)
39-39: Shebang is present but file is not executable
(EXE001)
39-39: Shebang should be at the beginning of the file
(EXE005)
54-54: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
61-61: CUOPT_PDLP_SOLVER_MODE
may be undefined, or defined from star imports
(F405)
63-63: CUOPT_TIME_LIMIT
may be undefined, or defined from star imports
(F405)
64-64: CUOPT_LOG_TO_CONSOLE
may be undefined, or defined from star imports
(F405)
PuLP_integration_example/Sudoku_pulp.ipynb
38-38: Shebang should contain python
, pytest
, or uv run
(EXE003)
38-38: Shebang is present but file is not executable
(EXE001)
38-38: Shebang should be at the beginning of the file
(EXE005)
39-39: Shebang should contain python
, pytest
, or uv run
(EXE003)
39-39: Shebang is present but file is not executable
(EXE001)
39-39: Shebang should be at the beginning of the file
(EXE005)
50-50: from pulp import *
used; unable to detect undefined names
(F403)
57-57: Ambiguous variable name: l
(E741)
63-63: LpProblem
may be undefined, or defined from star imports
(F405)
PuLP_integration_example/Simple_LP_pulp.ipynb
35-35: Shebang should contain python
, pytest
, or uv run
(EXE003)
35-35: Shebang is present but file is not executable
(EXE001)
35-35: Shebang should be at the beginning of the file
(EXE005)
36-36: Shebang should contain python
, pytest
, or uv run
(EXE003)
36-36: Shebang is present but file is not executable
(EXE001)
36-36: Shebang should be at the beginning of the file
(EXE005)
40-40: from pulp import *
used; unable to detect undefined names
(F403)
56-56: LpProblem
may be undefined, or defined from star imports
(F405)
56-56: LpMinimize
may be undefined, or defined from star imports
(F405)
57-57: LpVariable
may be undefined, or defined from star imports
(F405)
58-58: lpSum
may be undefined, or defined from star imports
(F405)
61-61: lpSum
may be undefined, or defined from star imports
(F405)
64-64: CUOPT
may be undefined, or defined from star imports
(F405)
67-67: value
may be undefined, or defined from star imports
(F405)
portfolio_optimization/cvar_portfolio_optimization.ipynb
116-116: f-string without any placeholders
Remove extraneous f
prefix
(F541)
routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
53-53: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
56-56: Redefinition of unused os
from line 46
Remove definition: os
(F811)
diet_optimization/diet_optimization_lp.ipynb
8-8: subprocess
call with shell=True
seems safe, but may be changed in the future; consider rewriting without shell
(S602)
8-8: Starting a process with a partial executable path
(S607)
15-15: Do not catch blind exception: Exception
(BLE001)
206-206: f-string without any placeholders
Remove extraneous f
prefix
(F541)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
64-64: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
67-67: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
36-36: Shebang should contain python
, pytest
, or uv run
(EXE003)
36-36: Shebang is present but file is not executable
(EXE001)
36-36: Shebang should be at the beginning of the file
(EXE005)
39-39: Shebang should contain python
, pytest
, or uv run
(EXE003)
39-39: Shebang is present but file is not executable
(EXE001)
39-39: Shebang should be at the beginning of the file
(EXE005)
42-42: from pulp import *
used; unable to detect undefined names
(F403)
45-45: LpProblem
may be undefined, or defined from star imports
(F405)
45-45: LpMaximize
may be undefined, or defined from star imports
(F405)
48-48: LpVariable
may be undefined, or defined from star imports
(F405)
49-49: LpVariable
may be undefined, or defined from star imports
(F405)
50-50: LpVariable
may be undefined, or defined from star imports
(F405)
66-66: CUOPT
may be undefined, or defined from star imports
(F405)
69-69: LpStatus
may be undefined, or defined from star imports
(F405)
73-73: value
may be undefined, or defined from star imports
(F405)
GAMSPy_integration_example/trnsport_cuopt.ipynb
58-58: Found useless expression. Either assign it to a variable or remove it.
(B018)
73-73: Found useless expression. Either assign it to a variable or remove it.
(B018)
PuLP_integration_example/Simple_MIP_pulp.ipynb
38-38: Shebang should contain python
, pytest
, or uv run
(EXE003)
38-38: Shebang is present but file is not executable
(EXE001)
38-38: Shebang should be at the beginning of the file
(EXE005)
39-39: Shebang should contain python
, pytest
, or uv run
(EXE003)
39-39: Shebang is present but file is not executable
(EXE001)
39-39: Shebang should be at the beginning of the file
(EXE005)
42-42: from pulp import *
used; unable to detect undefined names
(F403)
45-45: LpProblem
may be undefined, or defined from star imports
(F405)
45-45: LpMinimize
may be undefined, or defined from star imports
(F405)
48-48: LpVariable
may be undefined, or defined from star imports
(F405)
49-49: LpVariable
may be undefined, or defined from star imports
(F405)
62-62: CUOPT
may be undefined, or defined from star imports
(F405)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
38-38: Shebang should contain python
, pytest
, or uv run
(EXE003)
38-38: Shebang is present but file is not executable
(EXE001)
38-38: Shebang should be at the beginning of the file
(EXE005)
39-39: Shebang should contain python
, pytest
, or uv run
(EXE003)
39-39: Shebang is present but file is not executable
(EXE001)
39-39: Shebang should be at the beginning of the file
(EXE005)
52-52: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
38-38: Shebang should contain python
, pytest
, or uv run
(EXE003)
38-38: Shebang is present but file is not executable
(EXE001)
38-38: Shebang should be at the beginning of the file
(EXE005)
39-39: Shebang should contain python
, pytest
, or uv run
(EXE003)
39-39: Shebang is present but file is not executable
(EXE001)
39-39: Shebang should be at the beginning of the file
(EXE005)
54-54: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
🔇 Additional comments (1)
portfolio_optimization/cvar_portfolio_optimization.ipynb (1)
162-165
: Verify necessity of new dependencies.The installation commands were updated to include
nvidia-nvjitlink-cu12/cu13
andrapids-logger==0.1.19
. While these may be required dependencies, it would be helpful to verify they are actually needed for the notebook to run, or if they're pulled in automatically as transitive dependencies of cuopt.You can verify by checking cuOpt's installation documentation or testing if the notebook runs without explicitly installing these packages (since cuopt may already depend on them).
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
Outdated
Show resolved
Hide resolved
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
Outdated
Show resolved
Hide resolved
sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
Outdated
Show resolved
Hide resolved
sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
♻️ Duplicate comments (8)
GAMSPy_integration_example/trnsport_cuopt.ipynb (1)
63-105
: Add timeout + targeted exception handling to the GPU probeIf
nvidia-smi
stalls, this cell blocks indefinitely becausecheck_output
has no timeout. We should usesubprocess.run
with a short timeout and catchTimeoutExpired
so the failure is surfaced cleanly instead of hanging.- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected" @@ - except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display(HTML(f""" @@ - """)) + <p><b>Details:</b> {exc}</p> + """))last_mile_delivery/cvrptw_service_team_routing.ipynb (1)
71-111
: Guard the GPU probe against blockingnvidia-smi
calls
subprocess.check_output(["nvidia-smi"], …)
has no timeout, so a wedged driver leaves this cell hanging forever. It also never catchesTimeoutExpired
, so moving torun()
later would still bubble up. Usesubprocess.run
with a short timeout and handleTimeoutExpired
alongside the other expected failures.- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected" @@ - except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display(HTML(f""" @@ - """)) + <p><b>Details:</b> {exc}</p> + """))sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb (1)
53-92
: Avoid indefinite hangs incheck_gpu()
subprocess.check_output(["nvidia-smi"], …)
can block forever and we never catchTimeoutExpired
. Please move tosubprocess.run
with a short timeout and surface any failure in the UI so users see why the check failed.- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected" @@ - except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display(HTML(f""" @@ - """)) + <p><b>Details:</b> {exc}</p> + """))PuLP_integration_example/Sudoku_pulp.ipynb (1)
41-78
: Hardencheck_gpu()
against hangs and stale exceptions
subprocess.check_output(...)
runs without a timeout, so ifnvidia-smi
blocks (e.g., driver issues) the notebook cell never returns. We also never catchTimeoutExpired
, meaning that even if you switch torun(...)
later the code would still fall through to an uncaught exception. Please swap tosubprocess.run
with a short timeout and handle the expected exceptions explicitly.- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected" @@ - except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display(HTML(f""" @@ - """)) + <p><b>Details:</b> {exc}</p> + """))PuLP_integration_example/Simple_MIP_pulp.ipynb (1)
39-78
: Prevent the GPU check from hanging the notebookRunning
nvidia-smi
viasubprocess.check_output
without a timeout means any driver hiccup stalls execution indefinitely, andTimeoutExpired
is never caught. Please adopt the safer pattern (list argv, short timeout, explicit exception handling).- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected" @@ - except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display(HTML(f""" @@ - """)) + <p><b>Details:</b> {exc}</p> + """))routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb (1)
45-82
: Make the GPU check resilient to hungnvidia-smi
callsCalling
nvidia-smi
without a timeout is risky: any driver problem freezes the cell, andTimeoutExpired
isn’t caught. Please adopt the safe helper (argv list,timeout=…
, targeted exceptions, include failure detail).- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected" @@ - except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display(HTML(f""" @@ - """)) + <p><b>Details:</b> {exc}</p> + """))routing_optimization_over_server/cvrptw_service_team_routing.ipynb (1)
66-105
: Ensure the GPU check doesn’t hangThe helper still uses
subprocess.check_output
without a timeout and never catchesTimeoutExpired
. That can freeze the notebook ifnvidia-smi
misbehaves. Please switch tosubprocess.run
with a short timeout and extend the exception list.- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected" @@ - except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired) as exc: + display(HTML(f""" @@ - """)) + <p><b>Details:</b> {exc}</p> + """))PuLP_integration_example/Simple_LP_pulp.ipynb (1)
37-79
: Centralizecheck_gpu()
helper across notebooks.This identical function now lives in many notebooks inside the PR. Please move it into a shared utility (e.g.,
notebooks/utils/gpu_check.py
) and import it, so we maintain GPU messaging in one place instead of copy/pasting updates everywhere.
🧹 Nitpick comments (4)
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb (1)
27-78
: Add timeout to subprocess call for reliability.The GPU check could hang indefinitely if
nvidia-smi
becomes unresponsive. Consider adding a timeout parameter to prevent notebook freezing.Apply this change to make the subprocess call more robust:
- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() + output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT, timeout=3).decode()Also update the exception handling to catch
subprocess.TimeoutExpired
:- except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired, IndexError) as e:sample_lp_sever_notebooks/linear-programming.ipynb (1)
49-92
: Add timeout to subprocess call for reliability.The GPU check implementation matches the one in other notebooks but lacks a timeout parameter. This could cause the notebook to hang if
nvidia-smi
becomes unresponsive.Apply this change:
- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() + output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT, timeout=3).decode()And update exception handling:
- except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired, IndexError) as e:Note: This is a common pattern across multiple notebooks in this PR - consider applying this fix to all GPU check implementations.
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb (1)
31-82
: Add timeout to subprocess call for reliability.Same GPU check pattern as in other notebooks, missing the timeout parameter. This could cause the notebook to hang if
nvidia-smi
becomes unresponsive.Apply this change:
- output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() + output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT, timeout=3).decode()And update exception handling:
- except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired, IndexError) as e:intra-factory_transport/intra-factory_transport.ipynb (1)
58-68
: Hardcoded line index may be fragile.Line 62 assumes
nvidia-smi
output always has GPU info on line 3. This could break if the output format changes across driver versions or in edge cases (e.g., multi-GPU systems, different locale settings).Consider making the parsing more robust:
def check_gpu(): try: output = subprocess.check_output(["nvidia-smi"], shell=False, stderr=subprocess.STDOUT).decode() - lines = output.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + # Extract first line containing GPU info (typically starts with "| ") + lines = output.splitlines() + gpu_info = next((line for line in lines if line.strip().startswith("| ") and "NVIDIA" in line), "GPU detected") display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
GAMSPy_integration_example/trnsport_cuopt.ipynb
(3 hunks)PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_LP_pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_MIP_pulp.ipynb
(1 hunks)PuLP_integration_example/Sudoku_pulp.ipynb
(1 hunks)diet_optimization/diet_optimization_lp.ipynb
(1 hunks)diet_optimization/diet_optimization_milp.ipynb
(1 hunks)intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
(1 hunks)intra-factory_transport/intra-factory_transport.ipynb
(3 hunks)last_mile_delivery/cvrp_daily_deliveries.ipynb
(3 hunks)last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
(3 hunks)last_mile_delivery/cvrptw_service_team_routing.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
(2 hunks)routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
(2 hunks)routing_optimization_over_server/cvrptw_service_team_routing.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
(2 hunks)workforce_optimization/workforce_optimization_milp.ipynb
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- last_mile_delivery/cvrp_daily_deliveries.ipynb
🧰 Additional context used
🪛 Ruff (0.14.0)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
81-81: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
84-84: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
GAMSPy_integration_example/trnsport_cuopt.ipynb
67-67: Found useless expression. Either assign it to a variable or remove it.
(B018)
74-74: Found useless expression. Either assign it to a variable or remove it.
(B018)
89-89: Found useless expression. Either assign it to a variable or remove it.
(B018)
sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
53-53: Shebang should contain python
, pytest
, or uv run
(EXE003)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
70-70: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
77-77: CUOPT_PDLP_SOLVER_MODE
may be undefined, or defined from star imports
(F405)
79-79: CUOPT_TIME_LIMIT
may be undefined, or defined from star imports
(F405)
80-80: CUOPT_LOG_TO_CONSOLE
may be undefined, or defined from star imports
(F405)
workforce_optimization/workforce_optimization_milp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
140-140: Loop control variable shift
not used within loop body
(B007)
157-157: Loop control variable worker
not used within loop body
(B007)
204-204: f-string without any placeholders
Remove extraneous f
prefix
(F541)
276-276: Loop control variable shift
not used within loop body
(B007)
PuLP_integration_example/Simple_LP_pulp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
51-51: Shebang should contain python
, pytest
, or uv run
(EXE003)
51-51: Shebang is present but file is not executable
(EXE001)
51-51: Shebang should be at the beginning of the file
(EXE005)
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
56-56: from pulp import *
used; unable to detect undefined names
(F403)
72-72: LpProblem
may be undefined, or defined from star imports
(F405)
72-72: LpMinimize
may be undefined, or defined from star imports
(F405)
73-73: LpVariable
may be undefined, or defined from star imports
(F405)
74-74: lpSum
may be undefined, or defined from star imports
(F405)
77-77: lpSum
may be undefined, or defined from star imports
(F405)
80-80: CUOPT
may be undefined, or defined from star imports
(F405)
83-83: value
may be undefined, or defined from star imports
(F405)
intra-factory_transport/intra-factory_transport.ipynb
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
53-53: Shebang should contain python
, pytest
, or uv run
(EXE003)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
70-70: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
73-73: Redefinition of unused os
from line 63
Remove definition: os
(F811)
diet_optimization/diet_optimization_lp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
208-208: f-string without any placeholders
Remove extraneous f
prefix
(F541)
sample_lp_sever_notebooks/linear-programming.ipynb
53-53: Shebang should contain python
, pytest
, or uv run
(EXE003)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
routing_optimization_over_server/cvrptw_service_team_routing.ipynb
70-70: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
73-73: Redefinition of unused os
from line 62
Remove definition: os
(F811)
diet_optimization/diet_optimization_milp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
210-210: f-string without any placeholders
Remove extraneous f
prefix
(F541)
PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
52-52: Shebang should contain python
, pytest
, or uv run
(EXE003)
52-52: Shebang is present but file is not executable
(EXE001)
52-52: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
58-58: from pulp import *
used; unable to detect undefined names
(F403)
61-61: LpProblem
may be undefined, or defined from star imports
(F405)
61-61: LpMaximize
may be undefined, or defined from star imports
(F405)
64-64: LpVariable
may be undefined, or defined from star imports
(F405)
65-65: LpVariable
may be undefined, or defined from star imports
(F405)
66-66: LpVariable
may be undefined, or defined from star imports
(F405)
82-82: CUOPT
may be undefined, or defined from star imports
(F405)
85-85: LpStatus
may be undefined, or defined from star imports
(F405)
89-89: value
may be undefined, or defined from star imports
(F405)
PuLP_integration_example/Simple_MIP_pulp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
58-58: from pulp import *
used; unable to detect undefined names
(F403)
61-61: LpProblem
may be undefined, or defined from star imports
(F405)
61-61: LpMinimize
may be undefined, or defined from star imports
(F405)
64-64: LpVariable
may be undefined, or defined from star imports
(F405)
65-65: LpVariable
may be undefined, or defined from star imports
(F405)
78-78: CUOPT
may be undefined, or defined from star imports
(F405)
81-81: LpStatus
may be undefined, or defined from star imports
(F405)
82-82: value
may be undefined, or defined from star imports
(F405)
PuLP_integration_example/Sudoku_pulp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
66-66: from pulp import *
used; unable to detect undefined names
(F403)
73-73: Ambiguous variable name: l
(E741)
79-79: LpProblem
may be undefined, or defined from star imports
(F405)
82-82: LpVariable
may be undefined, or defined from star imports
(F405)
89-89: lpSum
may be undefined, or defined from star imports
(F405)
94-94: lpSum
may be undefined, or defined from star imports
(F405)
97-97: lpSum
may be undefined, or defined from star imports
(F405)
100-100: lpSum
may be undefined, or defined from star imports
(F405)
143-143: CUOPT
may be undefined, or defined from star imports
(F405)
146-146: LpStatus
may be undefined, or defined from star imports
(F405)
155-155: value
may be undefined, or defined from star imports
(F405)
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
63-63: Found useless expression. Either assign it to a variable or remove it.
(B018)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
68-68: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
70-70: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
53-53: Shebang should contain python
, pytest
, or uv run
(EXE003)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
53-53: Shebang should contain python
, pytest
, or uv run
(EXE003)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
Show resolved
Hide resolved
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (6)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb (1)
46-87
: Still need a boolean outcome fromcheck_gpu()
Past feedback asked for a reusable helper that can gate later cells, but the function still returns
None
. Please have it returnTrue
/False
and capture that flag so the notebook can bail out early when no GPU is present (apply across the other notebooks in this PR).display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> <pre>{gpu_info_escaped}</pre> </div> """)) + return True - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError): display(HTML(""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3> ... </div> """)) + return False - -check_gpu() +gpu_available = check_gpu()routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb (1)
46-87
: Return a flag from the GPU checkSame as in the delivery notebook:
check_gpu()
should returnTrue
/False
so downstream cells can short‑circuit when the GPU is missing. Please add the boolean return and capture it in a variable here too.sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb (1)
49-94
: Expose the GPU check result as a booleanTo keep the pattern consistent (and allow the rest of the notebook to halt cleanly), please have
check_gpu()
returnTrue
/False
and store that result before proceeding, just like requested earlier.PuLP_integration_example/Sudoku_pulp.ipynb (1)
42-83
: Let callers know whether a GPU was foundPlease follow the earlier guidance: have
check_gpu()
return a boolean and stash it (e.g.,gpu_available = check_gpu()
) so later cells can skip or abort when a GPU is unavailable.sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb (1)
50-95
: Propagate the GPU check resultFor consistency with the previously requested pattern, please return
True
/False
fromcheck_gpu()
and capture that value before running the rest of the notebook.intra-factory_transport/intra-factory_transport.ipynb (1)
119-121
: Invalid package versions already flagged.This issue was already identified in the previous review. The
rapids-logger==0.1.19
version is invalid and should berapids-logger==0.1.1
. Additionally, the cuopt and nvidia-nvjitlink packages should be pinned to specific versions for reproducibility.
🧹 Nitpick comments (21)
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb (1)
52-77
: Surface the actual nvidia-smi failure reasonRight now the failure branch only shows generic recovery steps, so users cannot tell whether the command timed out, the binary is missing, or some other error occurred. Capture a short, escaped snippet from
stderr
/str(e)
and render it so the banner contains the concrete failure reason.You can do something like:
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + error_message = getattr(e, "stderr", "") or str(e) + error_message = html.escape(error_message.strip()) + display(HTML(f""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3> <p>This notebook requires a <b>GPU runtime</b>.</p> <h4>If running in Google Colab:</h4> <ol> <li>Click on <b>Runtime → Change runtime type</b></li> <li>Set <b>Hardware accelerator</b> to <b>GPU</b></li> <li>Then click <b>Save</b> and <b>Runtime → Restart runtime</b>.</li> </ol> <h4>If running in Docker:</h4> <ol> <li>Ensure you have <b>NVIDIA Docker runtime</b> installed (<code>nvidia-docker2</code>)</li> <li>Run container with GPU support: <code>docker run --gpus all ...</code></li> <li>Or use: <code>docker run --runtime=nvidia ...</code> for older Docker versions</li> <li>Verify GPU access: <code>docker run --gpus all nvidia/cuda:12.0.0-base-ubuntu22.04 nvidia-smi</code></li> </ol> <p><b>Additional resources:</b></p> <ul> <li><a href="https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html" target="_blank">NVIDIA Container Toolkit Installation Guide</a></li> </ul> + <pre>{error_message}</pre> </div> - """)) + """))sample_lp_sever_notebooks/linear-programming.ipynb (1)
67-94
: Expose the underlying GPU check failureSame as the other notebook: the fallback UI hides the concrete error (timeout vs missing binary, etc.). Please render a sanitized excerpt from
stderr
/str(e)
so users immediately see whynvidia-smi
failed while still getting the remediation steps.Apply the same change as suggested in
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
to capture and display the escaped error message within the HTML banner.portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb (2)
83-95
: Avoid capturing and ignoring exception ase
.
as e
suggests the value will be used, but it’s discarded. Either log/emit the details frome
or drop the binding (except (...)
), so future readers aren’t misled and linters don’t flag it.
83-95
: Factor the duplicatedcheck_gpu()
helper.The exact same 30‑line
check_gpu()
function now lives in several notebooks. Please move it into a shared utility (or reuse an existing helper) and import it, so future fixes land in one place instead of N copies.diet_optimization/diet_optimization_milp.ipynb (2)
81-93
: Drop the unusedas e
binding.The exception object isn’t referenced; remove
as e
(or actually use it in the warning) to keep the handler tidy and lint‑clean.
81-93
: Centralize the shared GPU check helper.This notebook carries the same
check_gpu()
implementation as the others. Pull it into a common utility (or reuse an existing helper) so the code lives in a single, maintainable spot.portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb (2)
81-93
: Removeas e
if you’re not emitting it.The exception instance is ignored; either log it or drop the alias so the handler remains intentional and avoids unused-variable lint.
81-93
: Please de-dupecheck_gpu()
.This helper is now copy/pasted across multiple notebooks. Consider relocating it to a shared module or utility cell to keep maintenance overhead low.
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb (2)
54-95
: Unused exception binding — removeas e
.You capture the exception but never use it; drop the alias (or log it) to make the handler intentional and quiet linters.
54-95
: Factor the repeated GPU check into a shared helper.Same
check_gpu()
appears in the other notebooks; refactor it into a common utility so you only have to maintain it once.last_mile_delivery/cvrp_daily_deliveries.ipynb (2)
112-124
: Either use the exception object or dropas e
.Right now
as e
is unused; remove it (or log the message) so the handler is clean and lint-friendly.
112-124
: De-duplicate the GPU check helper.Identical
check_gpu()
code exists in several notebooks. Please move it to a shared utility so future tweaks land in one place.PuLP_integration_example/Simple_MIP_pulp.ipynb (1)
40-53
: Hardencheck_gpu()
: usenvidia-smi -L
, drop unusede
, refine parsing
- Prefer
["nvidia-smi", "-L"]
and show first line; it’s stable and concise.- Remove
IndexError
from except (you already guard the index).- Either use the exception in the HTML or drop
as e
to avoid unused var.Example patch:
- result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + result = subprocess.run(["nvidia-smi", "-L"], capture_output=True, text=True, timeout=5) result.check_returncode() - lines = result.stdout.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "nvidia-smi ran but returned no GPU lines." gpu_info_escaped = html.escape(gpu_info) @@ - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as exc: + display(HTML(f""" @@ - </div> - """)) + <p><b>Details:</b> {html.escape(str(exc))}</p> + </div> + """))Also applies to: 53-81
GAMSPy_integration_example/trnsport_cuopt.ipynb (1)
61-74
: Make GPU check more robust and remove unused variable
- Use
["nvidia-smi", "-L"]
and display the first line; avoid relying on line 3 of default output.- Drop
IndexError
from the except list.- Either display
exc
details or removeas e
.Patch:
- result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + result = subprocess.run(["nvidia-smi", "-L"], capture_output=True, text=True, timeout=5) result.check_returncode() - lines = result.stdout.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "nvidia-smi ran but returned no GPU lines." gpu_info_escaped = html.escape(gpu_info) @@ - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as exc: + display(HTML(f""" @@ - </div> - """)) + <p><b>Details:</b> {html.escape(str(exc))}</p> + </div> + """))Also applies to: 74-101
routing_optimization_over_server/cvrptw_service_team_routing.ipynb (1)
63-108
: Refine GPU check:-L
, safer parsing, no unused var
- Switch to
["nvidia-smi", "-L"]
, use the first line if present.- Remove
IndexError
from except.- Use
exc
in the message or dropas e
.- result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + result = subprocess.run(["nvidia-smi", "-L"], capture_output=True, text=True, timeout=5) @@ - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + gpu_info = lines[0] if lines else "nvidia-smi ran but returned no GPU lines." @@ - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as exc: + display(HTML(f""" @@ - </div> - """)) + <p><b>Details:</b> {html.escape(str(exc))}</p> + </div> + """))PuLP_integration_example/Simple_LP_pulp.ipynb (1)
41-82
: Improve GPU check UX and cleanup
- Use
["nvidia-smi", "-L"]
and display the first line for stability.- Remove
IndexError
handling and the unusede
variable (or surface details).- result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + result = subprocess.run(["nvidia-smi", "-L"], capture_output=True, text=True, timeout=5) @@ - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + gpu_info = lines[0] if lines else "nvidia-smi ran but returned no GPU lines." @@ - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as exc: + display(HTML(f""" @@ - </div> - """)) + <p><b>Details:</b> {html.escape(str(exc))}</p> + </div> + """))PuLP_integration_example/Production_Planning_Example_Pulp.ipynb (1)
36-80
: Hardencheck_gpu
invocation and surface error details.Right now we call
"nvidia-smi"
via a partial path and then drop the captured exception. That leaves us exposed to PATH hijacking and hides actionable diagnostics when the command fails. Let’s resolve the binary once withshutil.which
, abort early if it isn’t found, and keep the exception message available (HTML-escaped) for the fallback banner. That also lets us drop the unusede
.-import subprocess -import html -from IPython.display import display, HTML +import subprocess +import html +import shutil +from IPython.display import display, HTML @@ -def check_gpu(): +def check_gpu(): + nvidia_smi = shutil.which("nvidia-smi") + if not nvidia_smi: + display(HTML(""" + <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> + <h3>⚠️ GPU not detected!</h3> + <p><code>nvidia-smi</code> was not found on PATH.</p> + <p>Please install the NVIDIA drivers or use a GPU-enabled runtime.</p> + </div> + """)) + return try: - result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + result = subprocess.run( + [nvidia_smi, "--query-gpu=name,driver_version,cuda_version", "--format=csv,noheader"], + capture_output=True, + text=True, + timeout=5, + check=True, + ) - result.check_returncode() - lines = result.stdout.splitlines() + lines = [ln.strip() for ln in result.stdout.splitlines() if ln.strip()] gpu_info = lines[2] if len(lines) > 2 else "GPU detected" gpu_info_escaped = html.escape(gpu_info) display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> <pre>{gpu_info_escaped}</pre> </div> """)) - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as err: + err_msg = html.escape((err.stdout or "") + (err.stderr or "") or str(err)) + display(HTML(f""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3> <p>This notebook requires a <b>GPU runtime</b>.</p> @@ - </div> - """)) + <details><summary>Details</summary><pre>{err_msg}</pre></details> + </div> + """))diet_optimization/diet_optimization_lp.ipynb (1)
36-80
: Resolvenvidia-smi
path and expose failure diagnostics incheck_gpu
.We still invoke
"nvidia-smi"
directly from PATH and drop the exception payload. Please mirror the hardened pattern we’re using elsewhere: resolve the executable withshutil.which
, bail out early when it’s missing, and surface the captured stdout/stderr (HTML-escaped) inside the failure banner. That also lets us drop the unused exception binding.Same diff as suggested in the PuLP notebook will apply here (add
import shutil
, use the resolved path, pass the richer query flags, keepcheck=True
, and emit the escaped error text).workforce_optimization/workforce_optimization_milp.ipynb (2)
72-118
: Apply the hardenedcheck_gpu
pattern here as well.Same feedback as the other notebooks: resolve the
nvidia-smi
binary withshutil.which
, enablecheck=True
with the structured query, and surface the escaped error payload so the guidance banner carries actionable diagnostics. This also removes the unused exception variable. Reuse the diff shared on the PuLP notebook to keep the helper consistent across the repo.
31-67
: Strip committed outputs and reset execution counts before merging.This notebook still contains saved execution artefacts (e.g., the full
nvidia-smi
dump and solver logs) plus non-nullexecution_count
values. Please clear every cell’s outputs and resetexecution_count
tonull
(jupyter nbconvert --clear-output --inplace workforce_optimization/workforce_optimization_milp.ipynb
ornbstripout
works) so we avoid leaking environment specifics and keep diffs reviewable.Also applies to: 140-204
intra-factory_transport/intra-factory_transport.ipynb (1)
55-72
: Consider simplifying exception handling.The
IndexError
in the exception tuple on line 72 is redundant because line 64 already guards against it with thelen(lines) > 2
check. You can safely removeIndexError
from the caught exceptions.Apply this diff to simplify:
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as e:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
GAMSPy_integration_example/trnsport_cuopt.ipynb
(2 hunks)PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_LP_pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_MIP_pulp.ipynb
(1 hunks)PuLP_integration_example/Sudoku_pulp.ipynb
(1 hunks)diet_optimization/diet_optimization_lp.ipynb
(1 hunks)diet_optimization/diet_optimization_milp.ipynb
(1 hunks)intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
(1 hunks)intra-factory_transport/intra-factory_transport.ipynb
(2 hunks)last_mile_delivery/cvrp_daily_deliveries.ipynb
(2 hunks)last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
(2 hunks)last_mile_delivery/cvrptw_service_team_routing.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
(1 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
(1 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
(1 hunks)routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
(2 hunks)routing_optimization_over_server/cvrptw_service_team_routing.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
(2 hunks)workforce_optimization/workforce_optimization_milp.ipynb
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- last_mile_delivery/cvrptw_service_team_routing.ipynb
🧰 Additional context used
🪛 Ruff (0.14.0)
routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
69-69: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
72-72: Redefinition of unused os
from line 62
Remove definition: os
(F811)
routing_optimization_over_server/cvrptw_service_team_routing.ipynb
70-70: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
73-73: Redefinition of unused os
from line 62
Remove definition: os
(F811)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
81-81: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
84-84: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
86-86: Avoid specifying long messages outside the exception class
(TRY003)
PuLP_integration_example/Sudoku_pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
58-58: Shebang should contain python
, pytest
, or uv run
(EXE003)
58-58: Shebang is present but file is not executable
(EXE001)
58-58: Shebang should be at the beginning of the file
(EXE005)
69-69: from pulp import *
used; unable to detect undefined names
(F403)
76-76: Ambiguous variable name: l
(E741)
82-82: LpProblem
may be undefined, or defined from star imports
(F405)
85-85: LpVariable
may be undefined, or defined from star imports
(F405)
92-92: lpSum
may be undefined, or defined from star imports
(F405)
97-97: lpSum
may be undefined, or defined from star imports
(F405)
100-100: lpSum
may be undefined, or defined from star imports
(F405)
103-103: lpSum
may be undefined, or defined from star imports
(F405)
146-146: CUOPT
may be undefined, or defined from star imports
(F405)
149-149: LpStatus
may be undefined, or defined from star imports
(F405)
158-158: value
may be undefined, or defined from star imports
(F405)
sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
PuLP_integration_example/Simple_MIP_pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
58-58: Shebang should contain python
, pytest
, or uv run
(EXE003)
58-58: Shebang is present but file is not executable
(EXE001)
58-58: Shebang should be at the beginning of the file
(EXE005)
61-61: from pulp import *
used; unable to detect undefined names
(F403)
64-64: LpProblem
may be undefined, or defined from star imports
(F405)
64-64: LpMinimize
may be undefined, or defined from star imports
(F405)
67-67: LpVariable
may be undefined, or defined from star imports
(F405)
68-68: LpVariable
may be undefined, or defined from star imports
(F405)
81-81: CUOPT
may be undefined, or defined from star imports
(F405)
84-84: LpStatus
may be undefined, or defined from star imports
(F405)
85-85: value
may be undefined, or defined from star imports
(F405)
diet_optimization/diet_optimization_lp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
210-210: f-string without any placeholders
Remove extraneous f
prefix
(F541)
diet_optimization/diet_optimization_milp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
212-212: f-string without any placeholders
Remove extraneous f
prefix
(F541)
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
workforce_optimization/workforce_optimization_milp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
142-142: Loop control variable shift
not used within loop body
(B007)
159-159: Loop control variable worker
not used within loop body
(B007)
206-206: f-string without any placeholders
Remove extraneous f
prefix
(F541)
278-278: Loop control variable shift
not used within loop body
(B007)
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
66-66: Found useless expression. Either assign it to a variable or remove it.
(B018)
PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
58-58: from pulp import *
used; unable to detect undefined names
(F403)
61-61: LpProblem
may be undefined, or defined from star imports
(F405)
61-61: LpMaximize
may be undefined, or defined from star imports
(F405)
64-64: LpVariable
may be undefined, or defined from star imports
(F405)
65-65: LpVariable
may be undefined, or defined from star imports
(F405)
66-66: LpVariable
may be undefined, or defined from star imports
(F405)
82-82: CUOPT
may be undefined, or defined from star imports
(F405)
85-85: LpStatus
may be undefined, or defined from star imports
(F405)
89-89: value
may be undefined, or defined from star imports
(F405)
sample_lp_sever_notebooks/linear-programming.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
GAMSPy_integration_example/trnsport_cuopt.ipynb
5-5: Shebang should contain python
, pytest
, or uv run
(EXE003)
5-5: Shebang is present but file is not executable
(EXE001)
5-5: Shebang should be at the beginning of the file
(EXE005)
64-64: Found useless expression. Either assign it to a variable or remove it.
(B018)
71-71: Found useless expression. Either assign it to a variable or remove it.
(B018)
86-86: Found useless expression. Either assign it to a variable or remove it.
(B018)
PuLP_integration_example/Simple_LP_pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
59-59: from pulp import *
used; unable to detect undefined names
(F403)
75-75: LpProblem
may be undefined, or defined from star imports
(F405)
75-75: LpMinimize
may be undefined, or defined from star imports
(F405)
76-76: LpVariable
may be undefined, or defined from star imports
(F405)
77-77: lpSum
may be undefined, or defined from star imports
(F405)
80-80: lpSum
may be undefined, or defined from star imports
(F405)
83-83: CUOPT
may be undefined, or defined from star imports
(F405)
86-86: value
may be undefined, or defined from star imports
(F405)
intra-factory_transport/intra-factory_transport.ipynb
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
100-100: Found useless expression. Either assign it to a variable or remove it.
(B018)
sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
58-58: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
67-67: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
74-74: CUOPT_PDLP_SOLVER_MODE
may be undefined, or defined from star imports
(F405)
76-76: CUOPT_TIME_LIMIT
may be undefined, or defined from star imports
(F405)
77-77: CUOPT_LOG_TO_CONSOLE
may be undefined, or defined from star imports
(F405)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
65-65: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
53-53: Shebang is present but file is not executable
(EXE001)
53-53: Shebang should be at the beginning of the file
(EXE005)
54-54: Shebang should contain python
, pytest
, or uv run
(EXE003)
54-54: Shebang is present but file is not executable
(EXE001)
54-54: Shebang should be at the beginning of the file
(EXE005)
55-55: Shebang should contain python
, pytest
, or uv run
(EXE003)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
67-67: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
100-100: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
PuLP_integration_example/Simple_MIP_pulp.ipynb (1)
104-108
: Update rapids-logger pin and verify cuopt-cu13 availability
rapids-logger==0.1.19
exceeds the current stable version (0.1.1 on PyPI); verify this version is available or update the pin to0.1.1
.cuopt-cu13
is not available on standard PyPI (only via NVIDIA's PyPI index), so the--extra-index-url
approach is correct; however, document this or consider fallback guidance.- Unversioned
cuopt-cu12
andnvidia-nvjitlink-cu*
packages with--upgrade
are acceptable for rapid NVIDIA releases, but document the expected minimum versions for reproducibility.intra-factory_transport/intra-factory_transport.ipynb (1)
73-100
: Helpful GPU detection with clear user guidance.The error message provides comprehensive instructions for both Google Colab and Docker environments, along with relevant documentation links. The implementation correctly uses
html.escape()
to prevent potential XSS issues.Regarding the Ruff B018 warning on line 100: this is a false positive. In Jupyter notebooks, calling functions for their side effects (displaying output via
IPython.display
) is idiomatic and intentional.
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (7)
routing_optimization_over_server/cvrptw_service_team_routing.ipynb (1)
129-131
: Package version specifications already reviewed.The package versions in these pip install commands were discussed in a previous review and confirmed to be intentional per project requirements.
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb (1)
42-90
: LGTM with the same minor refinements as other notebooks.This
check_gpu()
implementation is consistent with the pattern across the PR and addresses prior security/timeout concerns.The same optional refinements apply here:
- Remove unnecessary
IndexError
from Line 60's exception tuple (already guarded on Line 51)- Remove unused exception variable
e
Apply this diff:
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError):last_mile_delivery/cvrptw_service_team_routing.ipynb (1)
71-119
: Consistent GPU check implementation across notebooks.This follows the same secure pattern established in other notebooks.
The same optional refinements apply:
- Remove
IndexError
from Line 89's exception tuple (redundant with Line 80's guard)- Remove unused exception variable
e
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError):GAMSPy_integration_example/trnsport_cuopt.ipynb (1)
57-105
: GPU detection follows the established pattern.Implementation is secure and user-friendly.
Apply the same optional refinements:
- Remove
IndexError
from Line 75's exception tuple (Line 66 guards it)- Remove unused
e
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError):PuLP_integration_example/Simple_LP_pulp.ipynb (1)
37-85
: GPU check implementation is consistent and secure.Same optional refinements:
- Remove
IndexError
from Line 55 (Line 46 guards it)- Remove unused
e
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError):PuLP_integration_example/Production_Planning_Example_Pulp.ipynb (1)
36-84
: GPU detection follows the secure pattern.Apply the same optional refinements:
- Remove
IndexError
from Line 54 (Line 45 guards it)- Remove unused
e
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError):workforce_optimization/workforce_optimization_milp.ipynb (1)
36-79
: GPU check implementation is secure and consistent.The explicit
shell=False
on Line 41 is redundant (it's the default) but harmless.Apply the same optional refinements:
- Remove
IndexError
from Line 50 (Line 43 guards it)- Remove unused
e
- except (subprocess.CalledProcessError, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, FileNotFoundError):Note on code duplication: The
check_gpu()
function is now duplicated across 20+ notebooks in this PR. While you've indicated this is per requirements, consider extracting it to a shared utility module (e.g.,utils/gpu_check.py
) in a future refactor to simplify maintenance and ensure consistency across updates. Based on learnings.
🧹 Nitpick comments (13)
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb (1)
50-97
: GPU check hardening looks good; consider extracting to shared utility.The
check_gpu()
function now properly addresses the security and robustness concerns from previous reviews:
- Uses
subprocess.run()
with a 5-second timeout- Guards against short output before indexing
- HTML-escapes the GPU info before embedding
- Catches specific exceptions including
TimeoutExpired
However, this function is duplicated across 20+ notebooks (per the AI summary), creating maintenance overhead. If the implementation needs updating, it would require changes in multiple places.
Consider extracting
check_gpu()
into a shared Python module (e.g.,utils/gpu_check.py
) that notebooks can import. This would centralize the logic and make future updates easier.Example shared module:
# utils/gpu_check.py import subprocess import html from IPython.display import display, HTML def check_gpu(): # ... (current implementation)Then in notebooks:
from utils.gpu_check import check_gpu check_gpu()intra-factory_transport/intra-factory_transport.ipynb (1)
102-102
: Consider capturing the return value for conditional execution.The
check_gpu()
call's boolean return value is currently unused. You might want to store it and use it to conditionally skip GPU-dependent cells if no GPU is detected.For example:
-check_gpu() +gpu_available = check_gpu() +# Later cells could check: if not gpu_available: skip or warnrouting_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb (2)
42-89
: LGTM! GPU check is now much safer.The previous concerns about
shell=True
, broad exception handling, and brittle line indexing have been addressed. The function now:
- Uses
subprocess.run(["nvidia-smi"], ...)
without shell- Includes proper timeout (5 seconds)
- Catches specific exceptions
- Safely bounds-checks before accessing line 2
- Escapes HTML output
Optional: Consider using
-L
flag for more reliable output.The plain
nvidia-smi
output format can vary across driver versions. Usingnvidia-smi -L
provides more consistent GPU listing that's easier to parse.If desired, apply this diff:
def check_gpu(): try: - result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + result = subprocess.run(["nvidia-smi", "-L"], capture_output=True, text=True, timeout=5) result.check_returncode() lines = result.stdout.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + gpu_info = lines[0] if len(lines) > 0 else "GPU detected" gpu_info_escaped = html.escape(gpu_info)Note: You can also remove
IndexError
from the exception tuple since the bounds checklen(lines) > 2
already prevents it.
108-110
: Add guidance for choosing between CUDA 12 and CUDA 13 variants.Two separate install commands for
cu12
andcu13
are provided, but users may not know which to choose. Consider adding a brief comment explaining when to use each variant (e.g., based on their CUDA version or driver compatibility).Example:
# Enable this in case you are running this in google colab or such places where cuOpt is not yet installed +# Use cu12 for CUDA 12.x environments, or cu13 for CUDA 13.x environments #!pip uninstall -y cuda-python cuda-bindings cuda-core #!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-server-cu12 nvidia-nvjitlink-cu12 rapids-logger==0.1.19 cuopt-sh-client #!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-server-cu13 nvidia-nvjitlink-cu13 rapids-logger==0.1.19 cuopt-sh-client
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb (1)
108-118
: Consider explicit imports for clarityThe wildcard import
from cuopt.linear_programming.solver.solver_parameters import *
makes it less clear where the parameter constants originate. While this works fine for example code, explicit imports improve code clarity.If you prefer explicit imports, you could replace line 117 with:
-from cuopt.linear_programming.solver.solver_parameters import * +from cuopt.linear_programming.solver.solver_parameters import ( + CUOPT_PDLP_SOLVER_MODE, + CUOPT_TIME_LIMIT, + CUOPT_LOG_TO_CONSOLE +)routing_optimization_over_server/cvrptw_service_team_routing.ipynb (1)
63-110
: Secure GPU check implementation looks good.The
check_gpu()
function addresses the security concerns from the previous review: it usessubprocess.run
with a list (noshell=True
), includes a timeout, escapes HTML output, and catches specific exceptions. The implementation is secure and functional.Optional refinements:
For slightly cleaner code, consider these minor adjustments:
- Use
check=True
parameter instead of callingcheck_returncode()
separately- Use
nvidia-smi -L
flag for simpler output parsingApply this diff for the refinements:
def check_gpu(): try: - result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) - result.check_returncode() + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) lines = result.stdout.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + gpu_info = lines[0] if lines else "GPU detected" gpu_info_escaped = html.escape(gpu_info)PuLP_integration_example/Sudoku_pulp.ipynb (1)
38-86
: Excellent GPU check implementation!This implementation properly addresses all the concerns from the previous review:
- ✅ Uses
subprocess.run()
with list arguments instead ofshell=True
- ✅ Includes a 5-second timeout
- ✅ Catches specific exceptions (
CalledProcessError
,TimeoutExpired
,FileNotFoundError
,IndexError
)- ✅ Safely guards
splitlines()[2]
access with length check- ✅ Escapes GPU info for safe HTML rendering
The comprehensive error message with instructions for both Colab and Docker environments is helpful.
Optional enhancement:
The exception variable
e
on line 56 is assigned but never used. Consider displaying exception details in the error message to aid debugging:- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: display(HTML(""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3> <p>This notebook requires a <b>GPU runtime</b>.</p> + <p style="font-size:0.9em;color:#666;"><b>Error details:</b> {}</p> <h4>If running in Google Colab:</h4> <ol> <li>Click on <b>Runtime → Change runtime type</b></li> <li>Set <b>Hardware accelerator</b> to <b>GPU</b></li> <li>Then click <b>Save</b> and <b>Runtime → Restart runtime</b>.</li> </ol> <h4>If running in Docker:</h4> <ol> <li>Ensure you have <b>NVIDIA Docker runtime</b> installed (<code>nvidia-docker2</code>)</li> <li>Run container with GPU support: <code>docker run --gpus all ...</code></li> <li>Or use: <code>docker run --runtime=nvidia ...</code> for older Docker versions</li> <li>Verify GPU access: <code>docker run --gpus all nvidia/cuda:12.0.0-base-ubuntu22.04 nvidia-smi</code></li> </ol> <p><b>Additional resources:</b></p> <ul> <li><a href="https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html" target="_blank">NVIDIA Container Toolkit Installation Guide</a></li> </ul> - </div> - """)) + </div> + """.format(html.escape(str(e)))))intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb (1)
27-83
: Good implementation of GPU detection with prior concerns addressed.The GPU check function properly uses
subprocess.run
with a list argument (no shell injection risk), includes a 5-second timeout, escapes HTML output, and provides clear user guidance. The previous security and reliability issues have been resolved.However, two minor refinements:
- Remove unnecessary
IndexError
from the exception handler (Line 52): You're already guarding against short output withif len(lines) > 2
on Line 43, soIndexError
cannot occur.- Remove unused exception variable: The caught exception
e
is never referenced.Apply this diff:
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError):sample_lp_sever_notebooks/linear-programming.ipynb (1)
55-59
: Prefer querying the GPU list directly for stability
nvidia-smi
’s default table output is verbose and its header layout has changed across driver releases. Continuing to grablines[2]
keeps the check fragile. Switching tonvidia-smi -L
and displaying the first returned line gives you the actual GPU descriptor while avoiding format churn.- result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) - result.check_returncode() - lines = result.stdout.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + result = subprocess.run( + ["nvidia-smi", "-L"], + check=True, + capture_output=True, + text=True, + timeout=5, + ) + lines = result.stdout.splitlines() + gpu_info = lines[0] if lines else "GPU detected"You can then drop the
IndexError
from theexcept
clause as it’s no longer reachable.diet_optimization/diet_optimization_lp.ipynb (2)
40-54
: Security improvements look good!The GPU check implementation properly addresses the security and error handling concerns from the previous review:
- ✅ Uses
subprocess.run
with list argument (no shell injection)- ✅ Has
timeout=5
to prevent hanging- ✅ Calls
check_returncode()
for explicit error detection- ✅ HTML-escapes output via
html.escape()
(line 46)- ✅ Checks bounds before indexing
lines[2]
(line 45)- ✅ Catches specific exceptions
Optional refinements (based on static analysis hints):
Resolve absolute path for nvidia-smi: Using a partial path like
"nvidia-smi"
could theoretically allow PATH manipulation. Consider usingshutil.which("nvidia-smi")
to resolve the absolute path.Remove unused exception variable: The exception
e
on line 54 is captured but never used. Either log it for debugging or remove the binding.Apply this optional diff:
+import shutil + def check_gpu(): try: - result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + nvidia_smi_path = shutil.which("nvidia-smi") + if not nvidia_smi_path: + raise FileNotFoundError("nvidia-smi not found in PATH") + result = subprocess.run([nvidia_smi_path], capture_output=True, text=True, timeout=5) result.check_returncode() lines = result.stdout.splitlines() gpu_info = lines[2] if len(lines) > 2 else "GPU detected" gpu_info_escaped = html.escape(gpu_info) display(HTML(f""" <div style="border:2px solid #4CAF50;padding:10px;border-radius:10px;background:#e8f5e9;"> <h3>✅ GPU is enabled</h3> <pre>{gpu_info_escaped}</pre> </div> """)) return True - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError): display(HTML("""
321-321
: Minor: Simplify f-string or add placeholder.The f-string on line 321 contains an inline conditional expression but no format placeholders, which static analysis flags as unusual style.
Consider either:
- Extract the expression to a variable for clarity:
-print(f"Problem type: {'MIP' if problem.IsMIP else 'LP'}") +problem_type = 'MIP' if problem.IsMIP else 'LP' +print(f"Problem type: {problem_type}")- Or remove the f-prefix:
-print(f"Problem type: {'MIP' if problem.IsMIP else 'LP'}") +print("Problem type: " + ('MIP' if problem.IsMIP else 'LP'))sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb (1)
50-97
: GPU check implementation is secure and functional.The implementation successfully addresses the previous security concerns: no shell injection risk, specific exception handling, timeout protection, and HTML escaping.
A few optional refinements for cleaner output and debugging:
- Use
nvidia-smi -L
flag for cleaner GPU listing (currently line 2 of full output is typically a separator line like|---------|---------|
)- Remove
IndexError
from the exception tuple since line 59 already guards withlen(lines) > 2
- Display exception details in the error HTML to aid debugging
Apply this diff if you'd like cleaner GPU output:
- result = subprocess.run(["nvidia-smi"], capture_output=True, text=True, timeout=5) + result = subprocess.run(["nvidia-smi", "-L"], capture_output=True, text=True, timeout=5) result.check_returncode() lines = result.stdout.splitlines() - gpu_info = lines[2] if len(lines) > 2 else "GPU detected" + gpu_info = lines[0] if lines else "nvidia-smi ran but returned no output" gpu_info_escaped = html.escape(gpu_info)And this diff to show exception details:
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as e: display(HTML(""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3> @@ -92,6 +92,7 @@ <li><a href="https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html" target="_blank">NVIDIA Container Toolkit Installation Guide</a></li> </ul> + <p><b>Debug info:</b> {html.escape(str(e))}</p> </div> """))PuLP_integration_example/Simple_MIP_pulp.ipynb (1)
55-81
: Surface the GPU check error details in the banner.Right now we catch the exception as
e
but drop it, so users never see whynvidia-smi
failed. Escaping and rendering the message gives actionable diagnostics (command not found, driver load error, timeout, etc.) while keeping the guidance you added.Apply this diff:
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: - display(HTML(""" + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e: + error_detail = html.escape(str(e) or "No additional error information was provided.") + display(HTML(f""" <div style="border:2px solid red;padding:15px;border-radius:10px;background:#ffeeee;"> <h3>⚠️ GPU not detected!</h3> <p>This notebook requires a <b>GPU runtime</b>.</p> <h4>If running in Google Colab:</h4> <ol> <li>Click on <b>Runtime → Change runtime type</b></li> <li>Set <b>Hardware accelerator</b> to <b>GPU</b></li> <li>Then click <b>Save</b> and <b>Runtime → Restart runtime</b>.</li> </ol> <h4>If running in Docker:</h4> <ol> <li>Ensure you have <b>NVIDIA Docker runtime</b> installed (<code>nvidia-docker2</code>)</li> <li>Run container with GPU support: <code>docker run --gpus all ...</code></li> <li>Or use: <code>docker run --runtime=nvidia ...</code> for older Docker versions</li> <li>Verify GPU access: <code>docker run --gpus all nvidia/cuda:12.0.0-base-ubuntu22.04 nvidia-smi</code></li> </ol> <p><b>Additional resources:</b></p> <ul> <li><a href="https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html" target="_blank">NVIDIA Container Toolkit Installation Guide</a></li> </ul> + <p><b>Error details:</b></p> + <pre>{error_detail}</pre> </div> - """)) + """)) return False
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
GAMSPy_integration_example/trnsport_cuopt.ipynb
(2 hunks)PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_LP_pulp.ipynb
(1 hunks)PuLP_integration_example/Simple_MIP_pulp.ipynb
(1 hunks)PuLP_integration_example/Sudoku_pulp.ipynb
(1 hunks)diet_optimization/diet_optimization_lp.ipynb
(1 hunks)diet_optimization/diet_optimization_milp.ipynb
(1 hunks)intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
(1 hunks)intra-factory_transport/intra-factory_transport.ipynb
(2 hunks)last_mile_delivery/cvrp_daily_deliveries.ipynb
(2 hunks)last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
(2 hunks)last_mile_delivery/cvrptw_service_team_routing.ipynb
(2 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
(1 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
(1 hunks)portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
(1 hunks)routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
(2 hunks)routing_optimization_over_server/cvrptw_service_team_routing.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/linear-programming.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
(2 hunks)sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
(2 hunks)workforce_optimization/workforce_optimization_milp.ipynb
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- last_mile_delivery/cvrp_daily_deliveries.ipynb
🧰 Additional context used
🪛 Ruff (0.14.0)
sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
60-60: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
PuLP_integration_example/Simple_LP_pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Consider moving this statement to an else
block
(TRY300)
21-21: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
61-61: from pulp import *
used; unable to detect undefined names
(F403)
77-77: LpProblem
may be undefined, or defined from star imports
(F405)
77-77: LpMinimize
may be undefined, or defined from star imports
(F405)
78-78: LpVariable
may be undefined, or defined from star imports
(F405)
79-79: lpSum
may be undefined, or defined from star imports
(F405)
82-82: lpSum
may be undefined, or defined from star imports
(F405)
85-85: CUOPT
may be undefined, or defined from star imports
(F405)
88-88: value
may be undefined, or defined from star imports
(F405)
diet_optimization/diet_optimization_milp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Consider moving this statement to an else
block
(TRY300)
21-21: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
214-214: f-string without any placeholders
Remove extraneous f
prefix
(F541)
PuLP_integration_example/Simple_MIP_pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Consider moving this statement to an else
block
(TRY300)
21-21: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
60-60: from pulp import *
used; unable to detect undefined names
(F403)
63-63: LpProblem
may be undefined, or defined from star imports
(F405)
63-63: LpMinimize
may be undefined, or defined from star imports
(F405)
66-66: LpVariable
may be undefined, or defined from star imports
(F405)
67-67: LpVariable
may be undefined, or defined from star imports
(F405)
80-80: CUOPT
may be undefined, or defined from star imports
(F405)
83-83: LpStatus
may be undefined, or defined from star imports
(F405)
84-84: value
may be undefined, or defined from star imports
(F405)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/03_advanced_topics.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
69-69: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
102-102: Avoid specifying long messages outside the exception class
(TRY003)
PuLP_integration_example/Production_Planning_Example_Pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Consider moving this statement to an else
block
(TRY300)
21-21: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
60-60: from pulp import *
used; unable to detect undefined names
(F403)
63-63: LpProblem
may be undefined, or defined from star imports
(F405)
63-63: LpMaximize
may be undefined, or defined from star imports
(F405)
66-66: LpVariable
may be undefined, or defined from star imports
(F405)
67-67: LpVariable
may be undefined, or defined from star imports
(F405)
68-68: LpVariable
may be undefined, or defined from star imports
(F405)
84-84: CUOPT
may be undefined, or defined from star imports
(F405)
87-87: LpStatus
may be undefined, or defined from star imports
(F405)
91-91: value
may be undefined, or defined from star imports
(F405)
GAMSPy_integration_example/trnsport_cuopt.ipynb
5-5: Shebang should contain python
, pytest
, or uv run
(EXE003)
5-5: Shebang is present but file is not executable
(EXE001)
5-5: Shebang should be at the beginning of the file
(EXE005)
66-66: Found useless expression. Either assign it to a variable or remove it.
(B018)
73-73: Found useless expression. Either assign it to a variable or remove it.
(B018)
88-88: Found useless expression. Either assign it to a variable or remove it.
(B018)
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
68-68: Found useless expression. Either assign it to a variable or remove it.
(B018)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
67-67: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
intra-factory_transport/intra-factory_transport.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
102-102: Found useless expression. Either assign it to a variable or remove it.
(B018)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
83-83: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
86-86: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
diet_optimization/diet_optimization_lp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Consider moving this statement to an else
block
(TRY300)
21-21: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
212-212: f-string without any placeholders
Remove extraneous f
prefix
(F541)
workforce_optimization/workforce_optimization_milp.ipynb
8-8: Starting a process with a partial executable path
(S607)
17-17: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
140-140: Loop control variable shift
not used within loop body
(B007)
157-157: Loop control variable worker
not used within loop body
(B007)
204-204: f-string without any placeholders
Remove extraneous f
prefix
(F541)
276-276: Loop control variable shift
not used within loop body
(B007)
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
60-60: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
PuLP_integration_example/Sudoku_pulp.ipynb
9-9: Starting a process with a partial executable path
(S607)
20-20: Consider moving this statement to an else
block
(TRY300)
21-21: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
58-58: Shebang is present but file is not executable
(EXE001)
58-58: Shebang should be at the beginning of the file
(EXE005)
59-59: Shebang should contain python
, pytest
, or uv run
(EXE003)
59-59: Shebang is present but file is not executable
(EXE001)
59-59: Shebang should be at the beginning of the file
(EXE005)
60-60: Shebang should contain python
, pytest
, or uv run
(EXE003)
60-60: Shebang is present but file is not executable
(EXE001)
60-60: Shebang should be at the beginning of the file
(EXE005)
71-71: from pulp import *
used; unable to detect undefined names
(F403)
78-78: Ambiguous variable name: l
(E741)
84-84: LpProblem
may be undefined, or defined from star imports
(F405)
87-87: LpVariable
may be undefined, or defined from star imports
(F405)
94-94: lpSum
may be undefined, or defined from star imports
(F405)
99-99: lpSum
may be undefined, or defined from star imports
(F405)
102-102: lpSum
may be undefined, or defined from star imports
(F405)
105-105: lpSum
may be undefined, or defined from star imports
(F405)
148-148: CUOPT
may be undefined, or defined from star imports
(F405)
151-151: LpStatus
may be undefined, or defined from star imports
(F405)
160-160: value
may be undefined, or defined from star imports
(F405)
sample_lp_sever_notebooks/linear-programming.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
60-60: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
69-69: from cuopt.linear_programming.solver.solver_parameters import *
used; unable to detect undefined names
(F403)
76-76: CUOPT_PDLP_SOLVER_MODE
may be undefined, or defined from star imports
(F405)
78-78: CUOPT_TIME_LIMIT
may be undefined, or defined from star imports
(F405)
79-79: CUOPT_LOG_TO_CONSOLE
may be undefined, or defined from star imports
(F405)
routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
71-71: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
74-74: Redefinition of unused os
from line 64
Remove definition: os
(F811)
routing_optimization_over_server/cvrptw_service_team_routing.ipynb
72-72: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
75-75: Redefinition of unused os
from line 64
Remove definition: os
(F811)
sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
60-60: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
🔇 Additional comments (20)
sample_lp_sever_notebooks/mixed-integer-linear-programming.ipynb (1)
117-119
: LGTM! Dependency installation expanded appropriately.The installation commands now include the necessary NVIDIA nvjitlink packages and rapids-logger for both CUDA 12 and 13 environments. Keeping these commented out is appropriate for notebooks where dependencies may already be installed.
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/02_backtesting.ipynb (2)
39-87
: ✅ Previous issues successfully addressed!The GPU check implementation now correctly handles all the concerns raised in the previous review:
- Timeout added:
subprocess.run([...], timeout=5)
prevents indefinite hangs- HTML escaping applied:
html.escape(gpu_info)
prevents HTML injection- TimeoutExpired handled: Included in the exception tuple on line 57
- Safe indexing: Fallback to "GPU detected" if lines are insufficient
The implementation is now safe and production-ready.
96-99
: LGTM! Clear installation instructions.The updated installation commands provide clear guidance for both CUDA 12 and CUDA 13 environments, including the necessary dependencies (nvidia-nvjitlink and rapids-logger).
diet_optimization/diet_optimization_milp.ipynb (3)
36-83
: Excellent GPU detection implementation!The
check_gpu()
function properly addresses all previous review concerns:
- Uses
subprocess.run()
withtimeout=5
to prevent hanging- Safely guards array indexing with
len(lines) > 2
- Escapes HTML output with
html.escape()
- Catches specific exceptions (
CalledProcessError
,TimeoutExpired
,FileNotFoundError
,IndexError
)- Provides clear, actionable guidance for both Colab and Docker environments
The implementation is robust and user-friendly.
92-95
: LGTM!The expanded pip install instructions now include the necessary dependencies (
nvidia-nvjitlink-cu12/cu13
andrapids-logger==0.1.19
) for both CUDA 12 and CUDA 13 variants. The commented format allows users to enable as needed.
106-468
: LGTM!The diet optimization problem implementation is well-structured and demonstrates proper use of the cuOpt Python API:
- Clear problem formulation with decision variables, objective function, and constraints
- Proper handling of both LP and MILP scenarios (integer variables for servings)
- Good demonstration of adding constraints dynamically
- Informative solution output with nutritional validation
The notebook provides a comprehensive example of GPU-accelerated optimization.
intra-factory_transport/intra-factory_transport.ipynb (2)
55-57
: LGTM! Appropriate imports for GPU detection.The imports are well-chosen:
subprocess
for runningnvidia-smi
,html
for XSS-safe output escaping, andIPython.display
for rendering user-friendly HTML banners in the notebook.
59-100
: Well-structured GPU detection with good security practices.The function correctly:
- Runs
nvidia-smi
with a reasonable 5-second timeout- Uses
html.escape()
to prevent XSS when displaying GPU info- Provides clear, actionable remediation steps for both Colab and Docker environments
- Handles multiple exception types defensively
sample_lp_sever_notebooks/linear-programming-with-datamodel.ipynb (2)
49-96
: LGTM! Previous security and reliability concerns have been addressed.The
check_gpu()
implementation now correctly:
- Uses
subprocess.run(["nvidia-smi"], ...)
with an explicit argument list instead of shell=True- Includes
timeout=5
to prevent hanging on wedged nvidia-smi processes- Defensively checks
len(lines) > 2
before indexing to avoid IndexError- HTML-escapes the GPU info via
html.escape()
to prevent markup injection- Catches specific exceptions including
subprocess.TimeoutExpired
,subprocess.CalledProcessError
, andFileNotFoundError
The error banner provides clear remediation guidance for both Colab and Docker environments.
115-117
: Installation instructions are clear and comprehensive.The expanded pip install commands appropriately cover both CUDA 12 and CUDA 13 variants with necessary dependencies including nvidia-nvjitlink and rapids-logger.
portfolio_optimization/cuFOLIO_portfolio_optimization/CVaR/01_optimization_with_cufolio.ipynb (4)
38-86
: LGTM - GPU detection properly hardenedThe GPU detection code follows security best practices with proper subprocess handling, specific exception catching, and safe output parsing. This addresses all concerns from previous reviews.
233-362
: Well-structured optimization setupThe CVaR parameter configuration, returns calculation, and optimization solving are clearly organized and well-documented. The code follows appropriate patterns for the cuOpt library.
427-434
: Good defensive programming in notebook contextChecking whether
sp500_optimal_portfolio
exists before using it is appropriate for an interactive notebook where cells might be run out of order. This provides a helpful user experience.
456-777
: Comprehensive examples with clear configurationThe efficient frontier generation and regime comparison code provides thorough examples with well-documented configuration parameters. The explicit parameter setting is helpful for educational purposes, making it easy for users to understand and modify the examples.
routing_optimization_over_server/cvrptw_service_team_routing.ipynb (1)
63-64
: Duplicate imports are acceptable in Jupyter notebooks.The static analysis tool flags duplicate imports of
subprocess
(lines 63, 177) andos
(lines 64, 180) across different notebook cells. This is a false positive for the notebook context—Jupyter cells commonly repeat imports to maintain cell independence and improve readability, and all cells share the same namespace at runtime.Also applies to: 177-177, 180-180
PuLP_integration_example/Sudoku_pulp.ipynb (4)
96-96
: LGTM - version pinning ensures reproducibility.Pinning PuLP to version 3.3.0 is good practice for ensuring consistent behavior across environments.
107-110
: LGTM - comprehensive installation instructions.The commented installation instructions properly include all necessary dependencies (
cuopt
,nvidia-nvjitlink
,rapids-logger
) and provide options for both CUDA 12 and CUDA 13 environments.
134-214
: LGTM - standard PuLP problem formulation.The Sudoku problem setup follows PuLP's official example structure. The star import (
from pulp import *
) and variable naming conventions are idiomatic for PuLP modeling code. Static analysis warnings about these can be safely ignored as they're standard practice in mathematical optimization frameworks.
236-256
: LGTM - clean solution implementation.The solution code properly invokes the cuOpt solver and displays the results in a well-formatted manner.
sample_lp_sever_notebooks/mixed-integer-linear-programming-with-datamodel.ipynb (1)
116-118
: Installation instructions are comprehensive and appropriate.The commented pip commands correctly include all necessary GPU-related dependencies (cuOpt variants for CUDA 12/13, nvjitlink, rapids-logger, and the client library). The commented format is appropriate for notebook portability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
last_mile_delivery/cvrptw_service_team_routing.ipynb (1)
137-139
: Clarify mutually exclusive installation options.The installation hints suggest installing both
cuopt-cu12
andcuopt-cu13
, but users should only install one variant based on their CUDA version. Consider adding a comment to clarify that these are alternative options, not sequential steps.Apply this diff to improve clarity:
# Enable this in case you are running this in google colab or such places where cuOpt is not yet installed #!pip uninstall -y cuda-python cuda-bindings cuda-core -#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu12 nvidia-nvjitlink-cu12 rapids-logger==0.1.19 -#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu13 nvidia-nvjitlink-cu13 rapids-logger==0.1.19 +# For CUDA 12.x: +#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu12 nvidia-nvjitlink-cu12 rapids-logger==0.1.19 +# For CUDA 13.x: +#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu13 nvidia-nvjitlink-cu13 rapids-logger==0.1.19last_mile_delivery/cvrp_daily_deliveries.ipynb (1)
67-114
: Consider extracting to a shared utility module.This function appears duplicated across 20+ notebooks. While duplication ensures each notebook is self-contained, it creates maintenance overhead—any future improvements must be applied to all copies.
Consider creating a shared utility module (e.g.,
cuopt_examples/utils/gpu_check.py
) that notebooks can import.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md
(1 hunks)intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
(2 hunks)last_mile_delivery/cvrp_daily_deliveries.ipynb
(3 hunks)last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
(3 hunks)last_mile_delivery/cvrptw_service_team_routing.ipynb
(3 hunks)routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
(3 hunks)routing_optimization_over_server/cvrptw_service_team_routing.ipynb
(3 hunks)
🧰 Additional context used
🪛 Ruff (0.14.0)
routing_optimization_over_server/cvrptw_service_team_routing.ipynb
72-72: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
75-75: Redefinition of unused os
from line 64
Remove definition: os
(F811)
routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
71-71: Redefinition of unused subprocess
from line 2
Remove definition: subprocess
(F811)
74-74: Redefinition of unused os
from line 64
Remove definition: os
(F811)
intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb
68-68: Found useless expression. Either assign it to a variable or remove it.
(B018)
92-92: Found useless expression. Either assign it to a variable or remove it.
(B018)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb
55-55: Shebang is present but file is not executable
(EXE001)
55-55: Shebang should be at the beginning of the file
(EXE005)
56-56: Shebang should contain python
, pytest
, or uv run
(EXE003)
56-56: Shebang is present but file is not executable
(EXE001)
56-56: Shebang should be at the beginning of the file
(EXE005)
57-57: Shebang should contain python
, pytest
, or uv run
(EXE003)
57-57: Shebang is present but file is not executable
(EXE001)
57-57: Shebang should be at the beginning of the file
(EXE005)
83-83: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
86-86: Probable insecure usage of temporary file or directory: "/tmp/data/C1_10_1.TXT"
(S108)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (15)
last_mile_delivery/cvrptw_benchmark_gehring_homberger.ipynb (3)
42-89
: LGTM! Previous review feedback properly addressed.The
check_gpu()
implementation successfully incorporates all the key improvements from previous reviews:
- Uses
subprocess.run()
with a 5-second timeout to prevent hanging- Properly escapes HTML output to prevent injection
- Returns a boolean to enable conditional notebook execution
- Catches
TimeoutExpired
along with other relevant exceptions- Avoids
shell=True
by using a list argumentThe implementation is now robust and production-ready for notebook use.
108-110
: Good flexibility for multiple CUDA versions.Providing both cu12 and cu13 installation paths as commented examples gives users clear options based on their environment. The uninstall step on line 108 is a good practice to avoid package conflicts.
121-121
: Removing--user
flag is appropriate for notebook environments.Installing packages without the
--user
flag is the correct approach for containerized notebook environments like Google Colab and Docker, where system-wide installation in the kernel is desired.last_mile_delivery/cvrptw_service_team_routing.ipynb (2)
71-118
: LGTM! Previous review concerns addressed.The GPU check implementation now follows best practices with proper subprocess handling (list args, timeout), specific exception handling, boundary guards, and HTML escaping. The previous review concerns have been fully resolved.
149-149
: LGTM! Improved Colab compatibility.Removing the
--user
flag aligns with standard Google Colab practices, as the environment is already user-specific and doesn't require explicit user-level installation.intra-factory_transport/cost_matrix_and_waypoint_graph_creation.ipynb (3)
34-81
: LGTM! GPU detection implementation is robust.The
check_gpu()
function properly addresses all concerns from the previous review:
- Uses
subprocess.run()
with list arguments (noshell=True
)- Includes a 5-second timeout
- Catches specific exceptions
- Guards array indexing with length check before accessing
lines[2]
- Uses
html.escape()
to prevent XSS- Provides clear, actionable guidance for both Colab and Docker environments
The implementation is secure, defensive, and user-friendly.
68-68
: Static analysis warnings are false positives.The Ruff warnings (B018) for lines 68 and 92 can be safely ignored:
- Line 68 is part of an HTML string literal, not executable Python code
- Line 92 is a Jupyter notebook magic command (
!pip install...
), which Ruff doesn't recognize in notebook contextsAlso applies to: 92-92
92-92
: LGTM! Removing--user
flag is appropriate for notebook environments.The removal of the
--user
flag is suitable for containerized environments and notebook platforms like Google Colab, where system-wide package installation is expected and necessary.README.md (1)
39-39
: LGTM! Appropriate for container context.Removing
--user
is correct for Docker containers where global package installation is preferred and isolation is already provided by the container itself.routing_optimization_over_server/cvrptw_service_team_routing.ipynb (2)
63-110
: LGTM! Secure and robust GPU detection.The
check_gpu()
implementation correctly addresses security and robustness:
- Uses list form for
subprocess.run
(no shell injection risk)- Includes timeout to prevent hanging
- Safely checks line count before indexing
- HTML-escapes output to prevent XSS
- Catches specific exceptions rather than blanket
except
- Returns boolean for programmatic use
129-131
: Note: Static analysis warnings are false positives.The linter flags
subprocess
andos
redefinitions, but these occur in different notebook cells (lines 63 and 177, lines 152 and later). In notebook context, each cell can independently import modules without conflict.routing_optimization_over_server/cvrptw_benchmark_gehring_homberger.ipynb (2)
42-89
: LGTM! Consistent and secure GPU check implementation.This implementation mirrors the secure pattern from
cvrptw_service_team_routing.ipynb
with proper subprocess handling, timeout, defensive parsing, HTML escaping, and specific exception handling. The duplication across notebooks is acceptable given their standalone nature.
108-110
: Note: Static analysis warnings are false positives in notebook context.Similar to the other notebook, linter warnings about
subprocess
andos
redefinitions (lines 155, 158) are false positives since imports occur in different notebook cells.last_mile_delivery/cvrp_daily_deliveries.ipynb (2)
67-84
: Security improvements look good!The implementation correctly addresses the previous security concerns:
- Uses list form
["nvidia-smi"]
instead of shell=True- Includes timeout protection (5 seconds)
- Properly escapes HTML output with
html.escape()
- Has bounds checking before accessing
lines[2]
146-146
: LGTM!The matplotlib installation is straightforward with appropriate quiet flag.
" </div>\n", | ||
" \"\"\"))\n", | ||
" return True\n", | ||
" except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e:\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant IndexError
from exception tuple.
The IndexError
exception is unnecessary because line 76 already includes bounds checking with if len(lines) > 2
. This exception can never be raised from the try block.
Apply this diff:
- except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e:
+ except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as e:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
" except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, IndexError) as e:\n", | |
" except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as e:\n", |
🤖 Prompt for AI Agents
In last_mile_delivery/cvrp_daily_deliveries.ipynb around line 85, the except
clause currently catches IndexError which is redundant because prior bounds
checking (if len(lines) > 2) prevents it; remove IndexError from the exception
tuple so the except only handles subprocess.CalledProcessError,
subprocess.TimeoutExpired, and FileNotFoundError. Update the except line
accordingly and run the cell to ensure no syntax errors.
"# Enable this in case you are running this in google colab or such places where cuOpt is not yet installed\n", | ||
"\n", | ||
"\n", | ||
"#!pip install --upgrade --extra-index-url https://pypi.nvidia.com --user cuopt-cu12" | ||
"#!pip uninstall -y cuda-python cuda-bindings cuda-core\n", | ||
"#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu12 nvidia-nvjitlink-cu12 rapids-logger==0.1.19\n", | ||
"#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu13 nvidia-nvjitlink-cu13 rapids-logger==0.1.19" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify mutually exclusive installation options.
The two install commands (cu12 vs cu13) are mutually exclusive, but there's no guidance on which to use. Users might uncomment both lines, causing package conflicts.
Consider adding a comment to clarify:
# Enable this in case you are running this in google colab or such places where cuOpt is not yet installed
#!pip uninstall -y cuda-python cuda-bindings cuda-core
+# Choose ONE of the following based on your CUDA version:
+# For CUDA 12.x, use:
#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu12 nvidia-nvjitlink-cu12 rapids-logger==0.1.19
+# For CUDA 13.x, use:
#!pip install --upgrade --extra-index-url=https://pypi.nvidia.com cuopt-cu13 nvidia-nvjitlink-cu13 rapids-logger==0.1.19
🤖 Prompt for AI Agents
In last_mile_delivery/cvrp_daily_deliveries.ipynb around lines 132 to 135,
clarify that the two pip install lines are mutually exclusive (cuopt-cu12 vs
cuopt-cu13) and instruct users to uncomment only the line that matches their
system CUDA toolkit (or Colab runtime) version; update the notebook comment to
state how to check CUDA version (e.g., nvcc --version or nvidia-smi) and to
never uncomment both lines to avoid conflicts, optionally suggesting a fallback
to the environment's default or linking to cuOpt compatibility docs.
Check for GPU while running notebook, if there is a error, provide details on what next steps can be taken.
Summary by CodeRabbit
New Features
Documentation
New Content
Chores