Implement python and C api for semi-continuous variables#1225
Open
hlinsen wants to merge 1 commit into
Open
Conversation
hlinsen
added
feature request
📝 WalkthroughWalkthroughThis PR adds comprehensive support for semi-continuous variables throughout cuOpt. It introduces the ChangesSemi-continuous variable type 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().
Comment on lines
+198
to
199
| variable_types_host[j] = detail::char_to_var_type(variable_types[j]); | ||
| } |
There was a problem hiding this comment.
Validate variable_types codes before conversion.
Unsupported type chars are currently coerced instead of rejected, which silently mutates caller input into a different model. Please return CUOPT_INVALID_ARGUMENT for unknown codes in both create paths before calling char_to_var_type(...).
Suggested patch pattern
- for (int j = 0; j < num_variables; j++) {
- variable_types_host[j] = detail::char_to_var_type(variable_types[j]);
- }
+ for (int j = 0; j < num_variables; j++) {
+ const char vtype = variable_types[j];
+ if (vtype != CUOPT_CONTINUOUS && vtype != CUOPT_INTEGER &&
+ vtype != CUOPT_SEMI_CONTINUOUS && vtype != 'B') {
+ delete problem_and_stream;
+ return CUOPT_INVALID_ARGUMENT;
+ }
+ variable_types_host[j] = detail::char_to_var_type(vtype);
+ }Also applies to: 259-260
🤖 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 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.
mlubin
reviewed
May 14, 2026
| } 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.
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