update doc for usage of incumbent utility function#947
Merged
rapids-bot[bot] merged 7 commits intoNVIDIA:release/26.04from Mar 21, 2026
Merged
update doc for usage of incumbent utility function#947rapids-bot[bot] merged 7 commits intoNVIDIA:release/26.04from
rapids-bot[bot] merged 7 commits intoNVIDIA:release/26.04from
Conversation
📝 WalkthroughWalkthroughDocstring for Problem.getIncumbentValues was expanded to clarify its role extracting incumbent variable values during Solve incumbent callbacks. Example and tutorial code were updated to pass problem and variable lists into IncumbentCallback and to use Problem.getIncumbentValues for per-variable reporting; example outputs were adjusted accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/cuopt/cuopt/linear_programming/problem.py (1)
1704-1720: LGTM! Documentation improvement is clear and helpful.The expanded docstring properly describes the method's purpose and parameters for the incumbent callback use case.
One optional enhancement: consider adding a
Returnssection to document the return type (List[float]) for completeness.📝 Optional: Add Returns section
vars : List[:py:class:`Variable`] List of variables to extract corresponding incumbent values. + + Returns + ------- + List[float] + List of incumbent values corresponding to the given variables. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cuopt/cuopt/linear_programming/problem.py` around lines 1704 - 1720, Add a Returns section to the getIncumbentValues docstring that documents the return type and contents (List[float] of incumbent values corresponding to the input vars). Update the docstring for the function getIncumbentValues to include a "Returns" heading describing that the function returns a list of floats (one value per variable in vars) and any brief detail about ordering or length matching vars.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@python/cuopt/cuopt/linear_programming/problem.py`:
- Around line 1704-1720: Add a Returns section to the getIncumbentValues
docstring that documents the return type and contents (List[float] of incumbent
values corresponding to the input vars). Update the docstring for the function
getIncumbentValues to include a "Returns" heading describing that the function
returns a list of floats (one value per variable in vars) and any brief detail
about ordering or length matching vars.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6486bf37-ef1e-44fb-be8d-0782be892a49
📒 Files selected for processing (1)
python/cuopt/cuopt/linear_programming/problem.py
Collaborator
|
@Iroy30 Also lets add this usage in one of the documentation example to showcase the usage. |
rgsl888prabhu
approved these changes
Mar 18, 2026
rgsl888prabhu
added
bug
Collaborator
|
/ok to test 38f0a8d |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.py`:
- Around line 31-37: Update the docstring expected output to match the runtime
prints: replace comma-separated "x=..., y=..." lines in the example output with
the space-separated pairs produced by the callback and final print blocks (e.g.,
align the "Incumbent 1: ..." and "Final solution: ..." lines to use the same
space-separated formatting shown at runtime); locate the output strings in the
example file (incumbent_solutions_example.py) where the docstring contains the
expected output block and make the formatting consistent with the actual prints
emitted by the callback and final print statements.
- Around line 116-119: Remove the unnecessary f-string and the unused enumerate
index in the final-solution print block: change the first print call to a plain
print (no f prefix) and iterate directly over problem.getVariables() instead of
using enumerate so the unused loop variable i is gone; keep using
var.VariableName and var.getValue() to print each variable and keep printing
problem.ObjValue for the final objective value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a6ae27a6-98c3-4f97-b14d-31b453213279
📒 Files selected for processing (3)
docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.pydocs/cuopt/source/cuopt-python/lp-qp-milp/lp-qp-milp-examples.rstskills/cuopt-lp-milp-api-python/assets/milp_basic/incumbent_callback.py
Comment on lines
+31
to
37
| Incumbent 1: x=36.0, y=41.0, cost: 303.00 | ||
|
|
||
| === Final Results === | ||
| Problem status: Optimal | ||
| Solve time: 0.16 seconds | ||
| Solve time: 0.27 seconds | ||
| Final solution: x=36.0, y=41.0 | ||
| Final objective value: 303.00 |
There was a problem hiding this comment.
Docstring expected output formatting is out of sync with actual prints.
The expected output uses comma-separated formatting (x=..., y=...), but the callback/final print blocks output space-separated pairs. Please align the docstring output with runtime output to avoid confusion.
✏️ Suggested docstring sync
- Incumbent 1: x=36.0, y=41.0, cost: 303.00
+ Incumbent 1: x=36.0 y=41.0 cost: 303.00
...
- Final solution: x=36.0, y=41.0
+ Final solution: x=36.0 y=41.0As per coding guidelines, docs/**/* changes should prioritize accuracy and consistency between examples and behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.py`
around lines 31 - 37, Update the docstring expected output to match the runtime
prints: replace comma-separated "x=..., y=..." lines in the example output with
the space-separated pairs produced by the callback and final print blocks (e.g.,
align the "Incumbent 1: ..." and "Final solution: ..." lines to use the same
space-separated formatting shown at runtime); locate the output strings in the
example file (incumbent_solutions_example.py) where the docstring contains the
expected output block and make the formatting consistent with the actual prints
emitted by the callback and final print statements.
Comment on lines
+116
to
+119
| print(f"Final solution: ", end=' ') | ||
| for i, var in enumerate(problem.getVariables()): | ||
| print(f"{var.VariableName}={var.getValue()} ", end=' ') | ||
| print(f"\nFinal objective value: {problem.ObjValue:.2f}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Ruff findings on the updated example file.
ruff check docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.pyRepository: NVIDIA/cuopt
Length of output: 1579
Fix Ruff violations F541 and B007 in final-solution print block.
Line 116 uses an f-string without placeholders, and line 117 introduces an unused loop variable i. Apply the suggested fix to remove the extraneous f prefix and eliminate the unused enumeration index.
Suggested fix
- print(f"Final solution: ", end=' ')
- for i, var in enumerate(problem.getVariables()):
- print(f"{var.VariableName}={var.getValue()} ", end=' ')
+ print("Final solution:", end="")
+ for var in problem.getVariables():
+ print(f" {var.VariableName}={var.getValue()}", end="")
print(f"\nFinal objective value: {problem.ObjValue:.2f}")🧰 Tools
🪛 Ruff (0.15.6)
[error] 116-116: f-string without any placeholders
Remove extraneous f prefix
(F541)
[warning] 117-117: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.py`
around lines 116 - 119, Remove the unnecessary f-string and the unused enumerate
index in the final-solution print block: change the first print call to a plain
print (no f prefix) and iterate directly over problem.getVariables() instead of
using enumerate so the unused loop variable i is gone; keep using
var.VariableName and var.getValue() to print each variable and keep printing
problem.ObjValue for the final objective value.
Member
Author
|
/ok to test b285473 |
Member
Author
|
/ok to test 176f996 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.py (1)
112-115: Remove unused loop variablei.The
enumerate()call introduces an unused index variablei. Since onlyvaris used, iterate directly overgetVariables().✏️ Suggested fix
- print("Final solution: ", end=" ") - for i, var in enumerate(problem.getVariables()): - print(f"{var.VariableName}={var.getValue()} ", end=" ") + print("Final solution:", end="") + for var in problem.getVariables(): + print(f" {var.VariableName}={var.getValue()}", end="") print(f"\nFinal objective value: {problem.ObjValue:.2f}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.py` around lines 112 - 115, The loop uses enumerate() but never uses the index variable `i`; change the iteration to loop directly over problem.getVariables() (e.g., replace "for i, var in enumerate(problem.getVariables()):" with "for var in problem.getVariables():") so only the used symbol `var` is bound and the unused `i` is removed; keep the surrounding prints and the final objective line unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.py`:
- Around line 112-115: The loop uses enumerate() but never uses the index
variable `i`; change the iteration to loop directly over problem.getVariables()
(e.g., replace "for i, var in enumerate(problem.getVariables()):" with "for var
in problem.getVariables():") so only the used symbol `var` is bound and the
unused `i` is removed; keep the surrounding prints and the final objective line
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b587708-0155-405b-8dca-7f0060b327f2
📒 Files selected for processing (3)
docs/cuopt/source/cuopt-python/lp-qp-milp/examples/incumbent_solutions_example.pydocs/cuopt/source/cuopt-python/lp-qp-milp/lp-qp-milp-examples.rstskills/cuopt-lp-milp-api-python/assets/milp_basic/incumbent_callback.py
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/cuopt/source/cuopt-python/lp-qp-milp/lp-qp-milp-examples.rst
- skills/cuopt-lp-milp-api-python/assets/milp_basic/incumbent_callback.py
Member
Author
|
/merge |
rapids-bot
bot
merged commit d7aa673
into
NVIDIA:release/26.04
707 of 724 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Issue
Closes #932
Checklist