Skip to content

Commit

Permalink
Fix regression when creating constraints comparing with ints or floats (
Browse files Browse the repository at this point in the history
#795)

* Fix regression when creating constraints comparing with ints or floats

* RHS should not be negated

* Have LpConstraint maintain its own constant to avoid using a shared constant when different constraints are instantiated with the same LpAffineExpression

* Format with black

* Fix implementation of LpConstraint.valid

* Fix type hint for Python 3.8

* Format with black

* LpConstraintVar.value should call value on LpConstraint and not LpAffineExpression
  • Loading branch information
MBradbury authored Feb 20, 2025
1 parent 76358a5 commit b842158
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 26 deletions.
4 changes: 3 additions & 1 deletion pulp/mps_lp.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,9 @@ def writeLP(LpProblem, filename, writeSOS=1, mip=1, max_length=100):
objName = LpProblem.objective.name
if not objName:
objName = "OBJ"
f.write(LpProblem.objective.asCplexLpAffineExpression(objName, constant=0))
f.write(
LpProblem.objective.asCplexLpAffineExpression(objName, include_constant=False)
)
f.write("Subject To\n")
ks = list(LpProblem.constraints.keys())
ks.sort()
Expand Down
75 changes: 51 additions & 24 deletions pulp/pulp.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,10 @@ def asCplexLpVariable(self):
s += f" <= {self.upBound:.12g}"
return s

def asCplexLpAffineExpression(self, name, constant=1):
return LpAffineExpression(self).asCplexLpAffineExpression(name, constant)
def asCplexLpAffineExpression(self, name, include_constant: bool = True):
return LpAffineExpression(self).asCplexLpAffineExpression(
name, include_constant
)

def __ne__(self, other):
if isinstance(other, LpElement):
Expand Down Expand Up @@ -846,7 +848,12 @@ def asCplexVariablesOnly(self, name: str):
line += [term]
return result, line

def asCplexLpAffineExpression(self, name: str, constant=1):
def asCplexLpAffineExpression(
self,
name: str,
include_constant: bool = True,
override_constant: float | None = None,
):
"""
returns a string that represents the Affine Expression in lp format
"""
Expand All @@ -856,11 +863,15 @@ def asCplexLpAffineExpression(self, name: str, constant=1):
term = f" {self.constant}"
else:
term = ""
if constant:
if self.constant < 0:
term = " - %s" % (-self.constant)
elif self.constant > 0:
term = f" + {self.constant}"
if include_constant:
constant = (
self.constant if override_constant is None else override_constant
)

if constant < 0:
term = " - %s" % (-constant)
elif constant > 0:
term = f" + {constant}"
if self._count_characters(line) + len(term) > const.LpCplexLPLineSize:
result += ["".join(line)]
line = [term]
Expand Down Expand Up @@ -1048,6 +1059,7 @@ def __init__(self, e=None, sense=const.LpConstraintEQ, name=None, rhs=None):
self.expr = (
e if isinstance(e, LpAffineExpression) else LpAffineExpression(e, name=name)
)
self.constant: float = 0.0
if rhs is not None:
self.constant -= rhs
self.sense = sense
Expand Down Expand Up @@ -1093,11 +1105,13 @@ def asCplexLpConstraint(self, name):
result = "%s\n" % "\n".join(result)
return result

def asCplexLpAffineExpression(self, name: str, constant=1):
def asCplexLpAffineExpression(self, name: str, include_constant: bool = True):
"""
returns a string that represents the Affine Expression in lp format
"""
return self.expr.asCplexLpAffineExpression(name, constant)
return self.expr.asCplexLpAffineExpression(
name, include_constant, override_constant=self.constant
)

def changeRHS(self, RHS):
"""
Expand All @@ -1114,7 +1128,7 @@ def __repr__(self):

def copy(self):
"""Make a copy of self"""
return LpConstraint(self, self.sense)
return LpConstraint(self, self.sense, rhs=-self.constant)

def emptyCopy(self):
return LpConstraint(sense=self.sense)
Expand All @@ -1127,15 +1141,19 @@ def addInPlace(self, other, sign=1):
"""
if isinstance(other, LpConstraint):
if self.sense * other.sense >= 0:
self.constant += other.constant
self.expr.addInPlace(other.expr, 1)
self.sense |= other.sense
else:
self.constant -= other.constant
self.expr.addInPlace(other.expr, -1)
self.sense |= -other.sense
elif isinstance(other, list):
for e in other:
self.addInPlace(e, sign)
else:
if isinstance(other, (int, float)):
self.constant += other * sign
self.expr.addInPlace(other, sign)
# raise TypeError, "Constraints and Expressions cannot be added"
return self
Expand Down Expand Up @@ -1206,8 +1224,8 @@ def __rdiv__(self, other):
c.expr = c.expr / other
return

def valid(self, eps=0):
val = self.expr.value()
def valid(self, eps=0) -> bool:
val = self.value()
if self.sense == const.LpConstraintEQ:
return abs(val) <= eps
else:
Expand Down Expand Up @@ -1262,23 +1280,18 @@ def name(self):
def name(self, v):
self.expr.name = v

@property
def constant(self):
return self.expr.constant

@constant.setter
def constant(self, v):
self.expr.constant = v

def isAtomic(self):
return self.expr.isAtomic()
return len(self) == 1 and self.constant == 0 and next(iter(self.values())) == 1

def isNumericalConstant(self):
return self.expr.isNumericalConstant()

def atom(self):
return self.expr.atom()

def __bool__(self):
return (float(self.constant) != 0.0) or (len(self) > 0)

def __len__(self):
return len(self.expr)

Expand All @@ -1300,6 +1313,20 @@ def values(self):
def items(self):
return self.expr.items()

def value(self) -> float | None:
s = self.constant
for v, x in self.items():
if v.varValue is None:
return None
s += v.varValue * x
return s

def valueOrDefault(self) -> float:
s = self.constant
for v, x in self.items():
s += v.valueOrDefault() * x
return s


class LpFractionConstraint(LpConstraint):
"""
Expand Down Expand Up @@ -1381,7 +1408,7 @@ def addVariable(self, var, coeff):
self.constraint.expr.addterm(var, coeff)

def value(self):
return self.constraint.expr.value()
return self.constraint.value()


class LpProblem:
Expand All @@ -1403,7 +1430,7 @@ def __init__(self, name="NoName", sense=const.LpMinimize):
warnings.warn("Spaces are not permitted in the name. Converted to '_'")
name = name.replace(" ", "_")
self.objective: None | LpAffineExpression = None
self.constraints: _DICT_TYPE[str, LpConstraint] = _DICT_TYPE()
self.constraints = _DICT_TYPE() # [str, LpConstraint]
self.name = name
self.sense = sense
self.sos1 = {}
Expand Down
24 changes: 23 additions & 1 deletion pulp/tests/test_pulp.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,16 @@ def test_continuous(self):
prob, self.solver, [const.LpStatusOptimal], {x: 4, y: -1, z: 6, w: 0}
)

def test_non_intermediate_var(self):
prob = LpProblem(self._testMethodName, const.LpMinimize)
x_vars = {
i: LpVariable(f"x{i}", lowBound=0, cat=LpContinuous) for i in range(3)
}
prob += lpSum(x_vars[i] for i in range(3)) >= 2
prob += lpSum(x_vars[i] for i in range(3)) <= 5
for elem in prob.constraints.values():
self.assertIn(elem.constant, [-2, -5])

def test_intermediate_var(self):
prob = LpProblem(self._testMethodName, const.LpMinimize)
x_vars = {
Expand All @@ -175,7 +185,19 @@ def test_intermediate_var(self):
prob += x >= 2
prob += x <= 5
for elem in prob.constraints.values():
self.assertIn(elem.constant, [2, 5])
self.assertIn(elem.constant, [-2, -5])

def test_comparison(self):
prob = LpProblem(self._testMethodName, const.LpMinimize)
x_vars = {
i: LpVariable(f"x{i}", lowBound=0, cat=LpContinuous) for i in range(3)
}
x = lpSum(x_vars[i] for i in range(3))

with self.assertRaises(TypeError):
prob += x > 2
with self.assertRaises(TypeError):
prob += x < 5

def test_continuous_max(self):
prob = LpProblem(self._testMethodName, const.LpMaximize)
Expand Down

0 comments on commit b842158

Please sign in to comment.