coin-or / python-mip

Python-MIP: collection of Python tools for the modeling and solution of Mixed-Integer Linear programs
Eclipse Public License 2.0
525 stars 92 forks source link

repr of Var and LinExpr #117

Closed jurasofish closed 4 years ago

jurasofish commented 4 years ago

Makes debugging a bit nicer by allowing you to see the value of variables in the console. Should this consider multiple solutions?

Screen Shot 2020-06-02 at 8 02 41 pm
h-g-s commented 4 years ago

Debugging is important. It would be good to provide a nice repr too (right now only str is overloaded). I did some search, the difference between both seems to be (https://stackoverflow.com/questions/1436703/difference-between-str-and-repr):

"The goal of repr is to be unambiguous" they recommend that eval(repr(c))==c, you know everything there is to know about c. In the case of var this is not really possible since vars are not directly created by the constructor, but with model.add_var ....

while "The goal of str is to be readable"

So, I think that the solution value (along more info) maybe should be better placed as information in str , what do you think ?

jurasofish commented 4 years ago

I'm cautious of changing the current str implementation as there might be some people relying on it to give the variable name or the LinExpr components. But, certainly, if we were to change it I think it could do with more info.

Yeah as you said I think it's a bit irrelevant to make eval(repr(var)) == var. Given that, it would make sense to make repr informative instead. This is similar to what pandas does, for example, where repr and str are pretty much the same. And since the repr of variables and linear expressions is currently the default object name and address I don't think we can really do worse :)

Perhaps something like the following would be good? I'm trying to keep it as concise as possible. Whether this is used for repr or str I'll leave up to you, though I think repr has an advantage as it's printed out by default when you evaluate variable in the console and won't introduce backwards compatibility issues.


>>> str(mip_var)  # Continuous variable with lb=1, ub=10 and no solution
'Var=None C[1, 10] "name"`
>>> str(mip_var)  # Continuous variable with lb=0, ub=inf and a solution
'Var=2.4 C[0, inf] "name"`
>>> str(mip_var)  # Integer variable
'Var=1 I[0, 5] "name"`
>>> str(mip_var)  # Binary variable
'Var=0 B[0, 1] "name"`
>>> str(mip_linexpr)
'LinExpr=2.5, 5 variables'
sebheger commented 4 years ago

In general, I would also say, that the usage for __repr__ makes more sense in this case. I would simply switch the code under __str__ to __repr__ (and remove __str__). And it is helpful to optionally include the solution (of the best known) value - if existing. @jurasofish remarks are going in the right direction, but I would suggest to just enhance the existing "str repr" you already have with the ideas in this PR.

h-g-s commented 4 years ago

Hi,

Yes, this additional information in repr would be great. The hex address of the object is really useless for debugging.

jurasofish commented 4 years ago

@h-g-s do you have any thoughts on other info that might be useful in there? Also any thoughts on what to do with __repr__ and __str__?

h-g-s commented 4 years ago

Hi, for LinExpr, if it has a sense, maybe something like

x1 + x2 + ... (more 3) <= 3
would be more readable, what do you think ? print the first variables, the sense and the rhs (-const)

CLAassistant commented 4 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 4 committers have signed the CLA.

:white_check_mark: jurasofish
:white_check_mark: bonelli
:x: h-g-s
:x: tuliotoffolo
You have signed the CLA already but the status is still pending? Let us recheck it.

jurasofish commented 4 years ago

I might close this for now - seems to me use cases for this library are a bit broader than just what I'm using. e.g. I never use variable or constraints names so i didn't consider them here.