pulp
pulp copied to clipboard
coin-or
pulp silently ignores 'double constraints'
In Python, it is possible to test for a variable to be within a range (ie. 1 <= a <= 5). Hence, this syntax is also legal in pulp constraints - however the first part is silently ignored.
I recommend one of three resolutions: a) allow the constraint to directly work as entered b) automatically split the constraint set in two sets - one for the first part and one for the second c) keep the current behavior but at least issue a warning
Test case: import pulp a = pulp.LpVariable('a', lowBound=0, cat=pulp.LpContinuous) milp = pulp.LpProblem("TC", pulp.LpMinimize) milp += a milp += 1 <= a <= 5 print(milp)
Expected behavior: a >= 1 and hence the optimal solution is a = 1 OR a warning is issued that "1 <=" will be ignored
Actual behavior:
First part of constraint milp += 1 <= a <= 5 (1 <= a) is silently ignored.
This to me sounds more like a feature request than a bug. However, the fact that it's not spitting out an error message is a bug in my opinion
To be clear, a workaround for this is to write the expression as two constraints, i.e., to add to the code above:
milp += 1 <= a
I've been trying to solve this but I'm unable to get both values in any of the methods in LpVariable and LpProblem. From what I've seen the order is as follows in the provided example:
1. a__ge__.(1) is executed and returns LpAffineExpression(a) >= 1, which is a constraint (let's call it c_1)
2. a__le__(5) is executed and returns LpAffineExpression(a) <= 5, which is a constraint (c_2)
3. milp.__iadd__(c_2) is called and adds the second one.
So, I have no idea how we can detect that there are two values being passed. Is there any internal python method like __ge__ but for interval comparison?
So apparently, we can "catch" the error if we stop evaluating affine expressions as a boolean. Making this change breaks many other tests of course since we do evaluate affine expressions as booleans in pulp. We could just replace all those references by some method like LpConstraint.is_filled() or something along those lines.
def __bool__(self):
raise const.PulpError('An affine expression has no boolean value.')
# return (float(self.constant) != 0.0) or (len(self) > 0)
hmm any idea why milp.__iadd__(c2) is called.
Apparently 1<=a<=3 is evaluated as 1<=a and a<=3
It's somehow explained here:
https://docs.python.org/3/reference/expressions.html#comparisons
I cite the relevant part:
Comparisons can be chained arbitrarily, e.g., x < y <= z is equivalent to x < y and y <= z, except that y is evaluated only once (but in both cases z is not evaluated at all when x < y is found to be false).
The thing is we cannot calculate: c_1 and c_2 in my previous example. Because then we would be (1) evaluating each of the two constraints as a boolean and (2) applying the and operator, which would return True but not add the constraints. That's why my solution was to not to let pulp evaluate constraints as boolean.
In any case, I do not see any clean way to allow the double constraint. And in order to raise an error it's still somewhat tricky.
A long long time ago, in a software called POAMS, I handled this issue by looking into the Python AST. You could still do that today, it is not as hard as it seems. But I thought that there was a change to semantics between 2 and 3.
On Fri, 25 Sep 2020 at 23:32, Franco Peschiera [email protected] wrote:
It's somehow explained here:
https://docs.python.org/3/reference/expressions.html#comparisons
I cite the relevant part:
Comparisons can be chained arbitrarily, e.g., x < y <= z is equivalent to x < y and y <= z, except that y is evaluated only once (but in both cases z is not evaluated at all when x < y is found to be false).
The thing is we cannot calculate: c_1 and c_2 in my previous example. Because then we would be (1) evaluating each of the two constraints as a boolean and (2) applying the and operator, which would return True but not add the constraints. That's why my solution was to not to let pulp evaluate constraints as boolean.
In any case, I do not see any clean way to allow the double constraint. And in order to raise an error it's still somewhat tricky.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/coin-or/pulp/issues/83#issuecomment-698930693, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGNOMVCE65QYCNIV4FYGFTSHSLXRANCNFSM4BFVNBQQ .
I think the problem with this isn't the comparison being added to you LpProblem but rather your definition of a LpElement.
The problem is that when you call the __le__ (or similar) operator you return self, but when multiple comparisons are called sequentially in this class it doesn't built upon them.
For example in this case if you call __ge__ then __le__ because only one value of self is ultimately returned, then it is always defined by the last call only ie a <= 5.
You can fix this by adding some memory / tracking mechanism into your comparison calls. For example here I have added a self.expressions list which gets each comparison appended. This isn't the nicest way to do it programmatically because it ultimately returns a list, I think you could do it nicer but it's just to demonstrate the point. Then test_double_constraint_respected shows that given this change the first constraint is respected and x is assigned the value of 1 instead of 0.
Happy to progress this further into a real pr if people are keen, just thought I would start by sharing the idea, what are people's thoughts?
