Implement python and C api for semi-continuous variables#1225
Open
hlinsen wants to merge 5 commits into
Open
Conversation
hlinsen
added
feature request
📝 WalkthroughWalkthroughThis pull request adds semi-continuous variable type support ( ChangesSemi-continuous variable support
🎯 3 (Moderate) | ⏱️ ~25 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
python/cuopt/cuopt/tests/linear_programming/test_python_API.py (1)
163-183: ⚡ Quick winAdd one test for the non-zero semi-continuous branch.
Line 163 currently validates only the
x = 0branch. Please add a case that forcesxinto[lb, ub]so both semi-continuous behaviors are covered.Proposed test addition
+def test_semi_continuous_variable_nonzero_branch(): + prob = Problem("Semi-continuous nonzero") + x = prob.addVariable(lb=5.0, ub=10.0, vtype=SEMI_CONTINUOUS, name="x") + prob.addConstraint(x >= 6.0) + prob.setObjective(x, sense=MINIMIZE) + + prob.solve() + + assert prob.Status.name == "Optimal" + assert prob.IsMIP + assert x.Value == pytest.approx(6.0) + assert prob.ObjValue == pytest.approx(6.0)As per coding guidelines, "Flag missing coverage for edge cases (empty, infeasible, unbounded, degenerate) when adding new code paths".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuopt/cuopt/tests/linear_programming/test_python_API.py` around lines 163 - 183, Add a second test that forces the non-zero semi-continuous branch by constructing a problem where x must lie in [lb, ub] (e.g., create a new Problem like "Semi-continuous-nonzero", add x = prob.addVariable(lb=5.0, ub=10.0, vtype=SEMI_CONTINUOUS, name="x") and y = prob.addVariable(lb=0.0, ub=1.0, vtype=CONTINUOUS, name="y"), impose a constraint that makes x >= lb such as prob.addConstraint(x + y == 6.0) so y=1 and x=5 is feasible, set the objective prob.setObjective(x, sense=MINIMIZE), solve with SolverSettings and assert prob.Status.name == "Optimal", prob.ObjValue == pytest.approx(5.0), x.Value == pytest.approx(5.0) and y.Value == pytest.approx(1.0) to cover the non-zero semi-continuous branch.)
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/src/mip_heuristics/presolve/semi_continuous.cu`:
- Around line 42-50: The current guard in ensure_constraint_bounds_populated
returns if either get_constraint_lower_bounds() OR get_constraint_upper_bounds()
is non-empty, which leaves the other side potentially empty and breaks later
rebuild logic; change the logic so it only returns when BOTH lower and upper are
populated, and if one side is missing populate the missing bounds to the correct
size (using get_constraint_bounds() when available or sizing from
get_row_types()) so that get_constraint_lower_bounds() and
get_constraint_upper_bounds() are both fully populated before proceeding; update
code in ensure_constraint_bounds_populated to check both sides, and when one is
empty create/fill it with appropriate default or copied values consistent with
existing get_constraint_bounds().
In `@cpp/src/pdlp/cuopt_c.cpp`:
- Around line 198-199: Validate the contents of the input variable_types array
before converting with detail::char_to_var_type: iterate the provided
variable_types and check each code against the supported set, and if any
unknown/unsupported char is found return CUOPT_INVALID_ARGUMENT immediately
(both create paths that populate variable_types_host from variable_types),
instead of coercing or calling detail::char_to_var_type; update the code around
the conversion sites (where variable_types_host[j] =
detail::char_to_var_type(variable_types[j]) is used) to perform the validation
first and only call char_to_var_type after all codes are confirmed valid.
---
Nitpick comments:
In `@python/cuopt/cuopt/tests/linear_programming/test_python_API.py`:
- Around line 163-183: Add a second test that forces the non-zero
semi-continuous branch by constructing a problem where x must lie in [lb, ub]
(e.g., create a new Problem like "Semi-continuous-nonzero", add x =
prob.addVariable(lb=5.0, ub=10.0, vtype=SEMI_CONTINUOUS, name="x") and y =
prob.addVariable(lb=0.0, ub=1.0, vtype=CONTINUOUS, name="y"), impose a
constraint that makes x >= lb such as prob.addConstraint(x + y == 6.0) so y=1
and x=5 is feasible, set the objective prob.setObjective(x, sense=MINIMIZE),
solve with SolverSettings and assert prob.Status.name == "Optimal",
prob.ObjValue == pytest.approx(5.0), x.Value == pytest.approx(5.0) and y.Value
== pytest.approx(1.0) to cover the non-zero semi-continuous branch.)
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e05ea914-f9eb-4929-b144-bb15cbad579f
📒 Files selected for processing (15)
cpp/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/cuopt_c.hcpp/include/cuopt/linear_programming/optimization_problem_utils.hppcpp/libmps_parser/src/mps_parser.cppcpp/src/mip_heuristics/presolve/semi_continuous.cucpp/src/pdlp/cpu_optimization_problem.cppcpp/src/pdlp/cuopt_c.cppcpp/src/pdlp/optimization_problem.cucpp/tests/linear_programming/c_api_tests/c_api_test.ccpp/tests/linear_programming/c_api_tests/c_api_tests.cppcpp/tests/linear_programming/c_api_tests/c_api_tests.hdocs/cuopt/source/cuopt-c/lp-qp-milp/lp-qp-milp-c-api.rstpython/cuopt/cuopt/linear_programming/problem.pypython/cuopt/cuopt/linear_programming/solver/solver.pypython/cuopt/cuopt/tests/linear_programming/test_python_API.py
Comment on lines
+42
to
+50
| template <typename i_t, typename f_t> | ||
| void ensure_constraint_bounds_populated(optimization_problem_t<i_t, f_t>& op_problem) | ||
| { | ||
| if (!op_problem.get_constraint_lower_bounds().is_empty() || | ||
| !op_problem.get_constraint_upper_bounds().is_empty()) { | ||
| return; | ||
| } | ||
| if (op_problem.get_row_types().is_empty() || op_problem.get_constraint_bounds().is_empty()) { | ||
| return; |
There was a problem hiding this comment.
Populate missing bounds when either side is absent (Line 45).
The guard at Line 45 returns when either lower or upper bounds is present. If only one side exists, the other remains empty, but later rebuild logic assumes both are fully populated and can mis-size bound arrays.
Proposed fix
template <typename i_t, typename f_t>
void ensure_constraint_bounds_populated(optimization_problem_t<i_t, f_t>& op_problem)
{
- if (!op_problem.get_constraint_lower_bounds().is_empty() ||
- !op_problem.get_constraint_upper_bounds().is_empty()) {
+ if (!op_problem.get_constraint_lower_bounds().is_empty() &&
+ !op_problem.get_constraint_upper_bounds().is_empty()) {
return;
}
if (op_problem.get_row_types().is_empty() || op_problem.get_constraint_bounds().is_empty()) {
return;
}As per coding guidelines, "Flag missing input validation at library and server boundaries".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/src/mip_heuristics/presolve/semi_continuous.cu` around lines 42 - 50, The
current guard in ensure_constraint_bounds_populated returns if either
get_constraint_lower_bounds() OR get_constraint_upper_bounds() is non-empty,
which leaves the other side potentially empty and breaks later rebuild logic;
change the logic so it only returns when BOTH lower and upper are populated, and
if one side is missing populate the missing bounds to the correct size (using
get_constraint_bounds() when available or sizing from get_row_types()) so that
get_constraint_lower_bounds() and get_constraint_upper_bounds() are both fully
populated before proceeding; update code in ensure_constraint_bounds_populated
to check both sides, and when one is empty create/fill it with appropriate
default or copied values consistent with existing get_constraint_bounds().
mlubin
reviewed
May 14, 2026
| @@ -250,7 +250,7 @@ BoundType convert(std::string_view str) | |||
| return LowerBoundIntegerVariable; | |||
| } else if (str == "UI") { | |||
| return UpperBoundIntegerVariable; | |||
| } else if (str == "SC" || str == "LC") { | |||
| } else if (str == "SC") { | |||
Contributor
There was a problem hiding this comment.
Is this change to mps_parser intentional?
Contributor
Author
There was a problem hiding this comment.
Yes. The semi continuous variables is always defined with the upper bound SC which is mandatory. LC shouldn't have been added.
rgsl888prabhu
approved these changes
May 15, 2026
Collaborator
rgsl888prabhu
left a comment
rgsl888prabhu
left a comment
There was a problem hiding this comment.
Rest looks good, I would suggest adding an example to docs under python. Not a blocking comment.
mlubin
approved these changes
May 18, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/src/pdlp/cuopt_c.cpp`:
- Around line 175-179: Add non-negative dimension guards for num_constraints and
num_variables before they are used: validate that num_constraints >= 0 and
num_variables >= 0 at the start of the API function (before the loop that checks
variable_types and before any access to constraint_matrix_row_offsets or sizing
of vectors); if either is negative return CUOPT_INVALID_ARGUMENT. Specifically,
add checks that guard uses of variable_types[num_variables],
constraint_matrix_row_offsets[num_constraints], and any vector/reserve calls
that take num_variables or num_constraints, ensuring early return on invalid
sizes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2c13229a-9462-46aa-a23b-9a92ffd1b955
📒 Files selected for processing (8)
cpp/include/cuopt/linear_programming/optimization_problem_utils.hppcpp/src/io/mps_parser.cppcpp/src/pdlp/cpu_optimization_problem.cppcpp/src/pdlp/cuopt_c.cppcpp/src/pdlp/optimization_problem.cudocs/cuopt/source/cuopt-python/lp-qp-milp/lp-qp-milp-examples.rstpython/cuopt/cuopt/linear_programming/problem.pypython/cuopt/cuopt/linear_programming/solver/solver.py
🚧 Files skipped from review as they are similar to previous changes (4)
- python/cuopt/cuopt/linear_programming/problem.py
- python/cuopt/cuopt/linear_programming/solver/solver.py
- cpp/src/pdlp/optimization_problem.cu
- cpp/src/pdlp/cpu_optimization_problem.cpp
Comment on lines
+175
to
+179
| for (int j = 0; j < num_variables; j++) { | ||
| if (!detail::is_valid_public_var_type_code(variable_types[j])) { | ||
| return CUOPT_INVALID_ARGUMENT; | ||
| } | ||
| } |
There was a problem hiding this comment.
Add non-negative dimension guards before using sizes/indices.
num_constraints and num_variables are not validated for negativity. With negative inputs, Line 188 / Line 254 index constraint_matrix_row_offsets[num_constraints], and vector sizing with num_variables can become unsafe.
Proposed fix
@@
- if (problem_ptr == nullptr || objective_coefficients == nullptr ||
+ if (problem_ptr == nullptr || objective_coefficients == nullptr ||
constraint_matrix_row_offsets == nullptr || constraint_matrix_column_indices == nullptr ||
constraint_matrix_coefficent_values == nullptr || constraint_sense == nullptr ||
rhs == nullptr || lower_bounds == nullptr || upper_bounds == nullptr ||
variable_types == nullptr) {
return CUOPT_INVALID_ARGUMENT;
}
+ if (num_constraints < 0 || num_variables < 0) { return CUOPT_INVALID_ARGUMENT; }
@@
- for (int j = 0; j < num_variables; j++) {
+ for (cuopt_int_t j = 0; j < num_variables; ++j) {
@@
- if (problem_ptr == nullptr || objective_coefficients == nullptr ||
+ if (problem_ptr == nullptr || objective_coefficients == nullptr ||
constraint_matrix_row_offsets == nullptr || constraint_matrix_column_indices == nullptr ||
constraint_matrix_coefficent_values == nullptr || constraint_lower_bounds == nullptr ||
constraint_upper_bounds == nullptr || variable_lower_bounds == nullptr ||
variable_upper_bounds == nullptr) {
return CUOPT_INVALID_ARGUMENT;
}
+ if (num_constraints < 0 || num_variables < 0) { return CUOPT_INVALID_ARGUMENT; }
@@
- for (int j = 0; j < num_variables; j++) {
+ for (cuopt_int_t j = 0; j < num_variables; ++j) {As per coding guidelines: "Flag missing input validation at library and server boundaries".
Also applies to: 239-245
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/src/pdlp/cuopt_c.cpp` around lines 175 - 179, Add non-negative dimension
guards for num_constraints and num_variables before they are used: validate that
num_constraints >= 0 and num_variables >= 0 at the start of the API function
(before the loop that checks variable_types and before any access to
constraint_matrix_row_offsets or sizing of vectors); if either is negative
return CUOPT_INVALID_ARGUMENT. Specifically, add checks that guard uses of
variable_types[num_variables], constraint_matrix_row_offsets[num_constraints],
and any vector/reserve calls that take num_variables or num_constraints,
ensuring early return on invalid sizes.
Comment on lines
+74
to
+87
| :download:`semi_continuous_example.py <examples/semi_continuous_example.py>` | ||
|
|
||
| .. literalinclude:: examples/semi_continuous_example.py | ||
| :language: python | ||
| :linenos: | ||
|
|
||
| The response is as follows: | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| Optimal solution found in 0.00 seconds | ||
| x = 0.0 | ||
| y = 1.0 | ||
| Objective value = 0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for the semi-continuous example file and display its content
# Check if the example file exists
if fd -t f 'semi_continuous_example.py' docs/cuopt/source/cuopt-python/lp-qp-milp/examples/; then
echo "=== File found, displaying content ==="
cat docs/cuopt/source/cuopt-python/lp-qp-milp/examples/semi_continuous_example.py
else
echo "ERROR: semi_continuous_example.py not found in expected location"
# Also search the entire repository
echo "=== Searching entire repository ==="
fd -t f 'semi_continuous_example.py'
fiRepository: NVIDIA/cuopt
Length of output: 223
🏁 Script executed:
#!/bin/bash
# Broader search for the semi_continuous_example.py file
echo "=== Searching entire repository for semi_continuous_example.py ==="
fd -t f 'semi_continuous_example.py'
echo ""
echo "=== Checking examples directory structure ==="
fd -t f . docs/cuopt/source/cuopt-python/lp-qp-milp/examples/ | head -20Repository: NVIDIA/cuopt
Length of output: 858
🏁 Script executed:
#!/bin/bash
# Check the exact content of the .rst file at lines 74-87
echo "=== Content of lp-qp-milp-examples.rst around lines 74-87 ==="
sed -n '74,87p' docs/cuopt/source/cuopt-python/lp-qp-milp/lp-qp-milp-examples.rstRepository: NVIDIA/cuopt
Length of output: 431
The referenced example file examples/semi_continuous_example.py does not exist in the repository.
The documentation at lines 74-87 includes both a :download: directive and a .. literalinclude:: directive pointing to examples/semi_continuous_example.py, but this file is not present in the repository. The examples directory contains only 9 files (expressions_constraints_example.py, incumbent_solutions_example.py, pdlp_warmstart_example.py, production_planning_example.py, qp_matrix_example.py, simple_lp_example.py, simple_milp_example.py, simple_qp_example.py, and solution_example.py), and semi_continuous_example.py is not among them.
This will cause the Sphinx documentation build to fail when it encounters the missing literalinclude directive. Either create the semi_continuous_example.py file with content that produces the documented output, or remove this example section from the documentation.
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.
This PR adds semi continuous to python and C APIs.
Closes: #1156