`X.isBinary()` is False in fixed binary variable

davidggphy commented 1 year ago

Details for the issue

X.isBinary() is False when the variable is binary but a value has been fixed.

What did you do?

import pulp
X = pulp.LpVariable("X",cat="Binary")
print(X.isBinary())
X.setInitialValue(1)
X.fixValue()
print(X.isBinary())

What did you expect to see?

True
True

What did you see instead?

True
False

Useful extra information

The info below often helps, please fill it out if you're able to. :)

What operating system are you using?

[x] Mac OS: ( _version: 13.0.1 )

I'm using python version:

[x] Other: 3.9.13

I installed PuLP via:

[x] pypi (python -m pip install pulp)

Did you also

[x] Tried out the latest github version: https://github.com/coin-or/pulp
[x] Searched for an existing similar issue: https://github.com/coin-or/pulp/issues?utf8=%E2%9C%93&q=is%3Aissue%20

MLopez-Ibanez commented 1 year ago

https://github.com/coin-or/pulp/blob/1366ad85fad1b5e8dda2b44cd143adeb206e2565/pulp/pulp.py#L653-L662

changes the bounds and:

https://github.com/coin-or/pulp/blob/1366ad85fad1b5e8dda2b44cd143adeb206e2565/pulp/pulp.py#L567-L568

checks the bounds (pulp doesn't a type LpBinary)

So fixing a variable stops it from being binary. I guess this depends on what the semantics of isBinary() should be. It is only used in a single place:

https://github.com/coin-or/pulp/blob/c0c391acaf75edd330bb81bc4ab0de7f0ee21b42/pulp/mps_lp.py#L325

If a binary variable with a fixed value is NOT a binary variable, then this should be documented but the code is correct. If a binary variable with a fixed value is binary, then isBinar() should use self._lowbound_original and self._upbound_original.

davidggphy commented 1 year ago

Thanks @MLopez-Ibanez

    def isBinary(self):
        return self.cat == const.LpInteger and self.lowBound == 0 and self.upBound == 1

    def isInteger(self):
        return self.cat == const.LpInteger

    def isFree(self):
        return self.lowBound is None and self.upBound is None

    def isConstant(self):
        return self.lowBound is not None and self.upBound == self.lowBound

My point is that, for the case of Integer variables, no matter if it is fixed or not the method isInteger returns True. In my opinion, it is more consistent if both return the type of the variable, independently of them being fixed, since for that you have the isConstant method.

coin-or / pulp