Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions python/cuopt/cuopt/linear_programming/problem.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ class Variable:
Value of the variable after solving.
ReducedCost : float
Reduced Cost after solving an LP problem.
MIPStart : float
Initial value (warm-start hint) for the variable. Defaults to NaN
(unset). Only used for a MIP problem.
"""

def __init__(
Expand All @@ -124,6 +127,7 @@ def __init__(
self.ReducedCost = float("nan")
self.VariableType = vtype
self.VariableName = vname
self.MIPStart = float("nan")

def getIndex(self):
"""
Expand Down Expand Up @@ -199,6 +203,20 @@ def getVariableName(self):
"""
return self.VariableName

def setMIPStart(self, val):
"""
Sets the MIP start (initial primal solution hint) value for the
variable. Use ``float("nan")`` to unset.
"""
self.MIPStart = float(val)

def getMIPStart(self):
"""
Returns the MIP start (initial primal solution hint) value of the
variable. Defaults to NaN when unset.
"""
return self.MIPStart
Comment on lines +206 to +218
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add type hints and complete API docstrings for new public methods.

setMIPStart/getMIPStart are new public APIs but currently lack type annotations and full docstring sections (Parameters, Returns, Raises).

Suggested patch
-    def setMIPStart(self, val):
+    def setMIPStart(self, val: float) -> None:
         """
-        Sets the MIP start (initial primal solution hint) value for the
-        variable. Use ``float("nan")`` to unset.
+        Set the MIP start (initial primal solution hint) value.
+
+        Parameters
+        ----------
+        val : float
+            Initial value for this variable. Use ``float("nan")`` to unset.
+
+        Raises
+        ------
+        ValueError
+            If ``val`` cannot be converted to float.
         """
-        self.MIPStart = float(val)
+        try:
+            self.MIPStart = float(val)
+        except (TypeError, ValueError) as e:
+            raise ValueError("MIPStart must be a numeric value or NaN") from e
 
-    def getMIPStart(self):
+    def getMIPStart(self) -> float:
         """
-        Returns the MIP start (initial primal solution hint) value of the
-        variable. Defaults to NaN when unset.
+        Get the MIP start (initial primal solution hint) value.
+
+        Returns
+        -------
+        float
+            The current hint value, or ``NaN`` when unset.
         """
         return self.MIPStart

As per coding guidelines, “Type hints on NEW public functions/classes” and “Docstring CONTENT on new public APIs — params, returns, raises”.

🤖 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/linear_programming/problem.py` around lines 206 - 218, The
new public methods setMIPStart and getMIPStart lack type hints and complete
docstrings; add type annotations (def setMIPStart(self, val: float) -> None and
def getMIPStart(self) -> float), update their docstrings to include Parameters,
Returns, and Raises sections, and validate input in setMIPStart (attempt
float(val) and raise a TypeError with a clear message if conversion fails) while
documenting that float("nan") unsets the value and that getMIPStart returns a
float (NaN when unset); reference the instance attribute MIPStart in the
docstrings so callers know the backing field.


def __neg__(self):
return LinearExpression([self], [-1.0], 0.0)

Expand Down Expand Up @@ -1428,6 +1446,7 @@ def _to_data_model(self):
self.lower_bound, self.upper_bound = np.zeros(n), np.zeros(n)
self.var_type = np.empty(n, dtype="S1")
self.var_names = []
self.mip_start = np.empty(n, dtype=np.float64)

for j in range(n):
self.objective[j] = self.vars[j].getObjectiveCoefficient()
Expand All @@ -1438,6 +1457,7 @@ def _to_data_model(self):
if var_name == "":
var_name = "C" + str(self.vars[j].index)
self.var_names.append(var_name)
self.mip_start[j] = self.vars[j].MIPStart

# Initialize datamodel
dm.set_csr_constraint_matrix(
Expand All @@ -1464,6 +1484,9 @@ def _to_data_model(self):
dm.set_row_names(self.row_names)
dm.set_problem_name(self.Name)

if self.mip_start.size > 0 and not np.all(np.isnan(self.mip_start)):
dm.set_initial_primal_solution(self.mip_start)
Comment on lines +1487 to +1488
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard against forwarding partially-NaN MIP starts to the data model.

Current condition forwards self.mip_start when any entry is non-NaN, so vectors like [5.0, NaN, ...] are passed through. That can inject invalid numeric values into solver initialization.

Suggested patch
-        if self.mip_start.size > 0 and not np.all(np.isnan(self.mip_start)):
-            dm.set_initial_primal_solution(self.mip_start)
+        if self.mip_start.size > 0 and not np.all(np.isnan(self.mip_start)):
+            if np.any(np.isnan(self.mip_start)):
+                raise ValueError(
+                    "MIPStart must be provided for all variables or left unset for all."
+                )
+            dm.set_initial_primal_solution(self.mip_start)
📝 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.

Suggested change
if self.mip_start.size > 0 and not np.all(np.isnan(self.mip_start)):
dm.set_initial_primal_solution(self.mip_start)
if self.mip_start.size > 0 and not np.all(np.isnan(self.mip_start)):
if np.any(np.isnan(self.mip_start)):
raise ValueError(
"MIPStart must be provided for all variables or left unset for all."
)
dm.set_initial_primal_solution(self.mip_start)
🤖 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/linear_programming/problem.py` around lines 1487 - 1488,
The current check forwards self.mip_start if any entry is non-NaN, allowing
partially-NaN vectors; change the guard to only forward when all entries are
finite (no NaN/Inf) — e.g., replace the condition on self.mip_start with a check
using np.all(np.isfinite(self.mip_start)) while still checking size > 0, and
only call dm.set_initial_primal_solution(self.mip_start) when that holds;
reference self.mip_start and dm.set_initial_primal_solution to locate the
change.


self.model = dm

def update(self):
Expand Down
41 changes: 41 additions & 0 deletions python/cuopt/cuopt/tests/linear_programming/test_python_API.py
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,47 @@ def test_warm_start():
)


def test_mip_start():
# Build a small MIP and feed it an (already-optimal) MIP start through
# the new Variable.MIPStart attribute. We verify:
# - the attribute defaults to NaN
# - setMIPStart / direct assignment both work
# - the values reach the data model via set_initial_primal_solution
# - solving still produces the optimal result
# - leaving every MIPStart as NaN does not set an initial primal sol
prob = Problem("MIP_start")
x = prob.addVariable(lb=0, ub=10, vtype=INTEGER, name="x")
y = prob.addVariable(lb=0, ub=10, vtype=INTEGER, name="y")

assert math.isnan(x.getMIPStart())
assert math.isnan(y.MIPStart)

prob.addConstraint(x + y <= 10, name="c1")
prob.addConstraint(x - y >= 0, name="c2")
prob.setObjective(x + 2 * y, sense=MAXIMIZE)

x.setMIPStart(5)
y.MIPStart = 5.0

assert x.getMIPStart() == 5.0
assert y.getMIPStart() == 5.0

prob._to_data_model()
initial_primal = prob.model.get_initial_primal_solution()
assert len(initial_primal) == 2
assert initial_primal[0] == pytest.approx(5.0)
assert initial_primal[1] == pytest.approx(5.0)

settings = SolverSettings()
settings.set_parameter("time_limit", 5)
prob.solve(settings)

assert prob.Status.name == "Optimal"
assert prob.ObjValue == pytest.approx(15)
assert x.Value == pytest.approx(5)
assert y.Value == pytest.approx(5)

Comment on lines +485 to +524
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add edge-case assertion for the all-NaN branch.

This test validates the “set hint” path, but not the “all hints unset” path introduced by the new branch in _to_data_model. Please add a case where all MIPStart remain NaN and assert no initial primal solution is set.

As per coding guidelines, “Flag missing coverage for edge cases (empty, infeasible, unbounded, degenerate) when adding new code paths” and for python/**/tests/**, focus on edge cases.

🤖 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
485 - 524, Add an additional edge-case assertion in test_mip_start to exercise
the "all-NaN" branch of Problem._to_data_model: create a scenario where
x.MIPStart and y.MIPStart remain NaN (do not call setMIPStart or assign), call
prob._to_data_model(), then call prob.model.get_initial_primal_solution() and
assert that no initial primal solution is set (e.g., assert the returned value
is None or an empty sequence / is falsy). Reference Variable.MIPStart,
test_mip_start, _to_data_model, and get_initial_primal_solution when locating
where to add this check.


def test_problem_update():
prob = Problem()
x1 = prob.addVariable(vtype=INTEGER, lb=0, name="x1")
Expand Down
Loading