More error checking in MixedIntegerLinearProgram

[mkoeppe]

The get_values method silently ignored non-variables.

CC: @dimpase @nathanncohen

Component: numerical

Author: Matthias Koeppe

Branch/Commit: 7d7aeed

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/20304

[mkoeppe]

Branch: u/mkoeppe/more_error_checking_in_mixedintegerlinearprogram

[mkoeppe]

comment:2

The get_values method silently ignored non-variables.

---

New commits:

dc3895b

MixedIntegerLinearProgram.get_values: Input checking

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Commit: dc3895b

[videlec]

Reviewer: Vincent Delecroix

[videlec]

comment:4

Please use new style error ValueError(msg) instead of ValueError, msg (Python 3 compatibility).

Would be better to use repr rather than str. The output of "{!r}".format(l) is nicer than str(l). As you can see

sage: l = 1
sage: str(l)
'1'
sage: "{!r}".format(l)
'1'
sage: l = '1'
sage: str(l)
'1'
sage: "{!r}".format(l)
"'1'"

The following are not caught correctly

sage: M1 = MixedIntegerLinearProgram()
sage: M2 = MixedIntegerLinearProgram()
sage: x = M1.new_variable()
sage: y = M1.new_variable()
sage: z = M2.new_variable()
sage: M2.add_constraint(z[0] <= 5)
sage: M2.solve()
0.0
sage: M2.get_values(x)              # should be a ValueError
Traceback (most recent call last):
...
KeyError: x_0
sage: M2.get_values(y)              # should be a ValueError
{}

[videlec]

comment:5

And if the input is not a variable it would be better to raise a TypeError rather than a ValueError.

[sagetrac-git]

Changed commit from dc3895b to 4290bde

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

4290bde

MixedIntegerLinearProgram: use new style error ValueError(msg) instead of ValueError, msg (Python 3 compatibility).

[sagetrac-git]

Changed commit from 4290bde to 13e82d9

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

67a263d	MIPVariable: New method mip
13e82d9	MixedIntegerLinearProgram.get_values: Also check that a MIPVariable is from the right MIP

[videlec]

comment:9

Nice.

Last thing: the first error should be a ValueError. The type is indeed the good one!

[mkoeppe]

comment:10

Should I then make it a ValueError also when it's a string, but not one that names a variable?
(If so, what's the right way to test if it is a string?)

[videlec]

comment:11

Replying to @mkoeppe:

Should I then make it a ValueError also when it's a string, but not one that names a variable?
(If so, what's the right way to test if it is a string?)

Not necessarily (one can assume that the "good" way of calling this function is with a MIP variable). Though, to test if the object x is a string you can use isinstance(x, str) (which returns a boolean).

[mkoeppe]

comment:12

I was confused, it actually does not accept strings -- only MIPVariable objects.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

7d7aeed

MixedIntegerLinearProgram.get_values: Raise TypeError when not a MIPVariable, ValueError when from the wrong MIP

[sagetrac-git]

Changed commit from 13e82d9 to 7d7aeed

[mkoeppe]

comment:15

Thanks for reviewing!

[vbraun]

Changed branch from u/mkoeppe/more_error_checking_in_mixedintegerlinearprogram to 7d7aeed