PPLBackend: Add support for integer variables

[mkoeppe]

PPL's solver is a rational MIP solver.
Its support for integer variables should be exposed in Sage.
#20351 does this for the PPL wrapper classes.

This ticket updates the PPLBackend class.

Depends on #20351
Depends on #20303

CC: @dimpase @videlec @jdemeyer

Component: numerical

Author: Matthias Koeppe

Branch/Commit: 144a970

Reviewer: Dima Pasechnik

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

[mkoeppe]

Changed dependencies from #20351 to #20351,#20303

[mkoeppe]

Branch: u/mkoeppe/pplbackend__add_support_for_integer_variables

[mkoeppe]

comment:3

Currently this branch is on top of the branches for #20351,#20303.

Needs review.

---

Last 10 new commits:

9f35b65	Wrap MIP_Problem.add_to_integer_space_dimensions
39473c9	Fix and FIXME
bceca77	CVXOPTBackend: Use 'is None' instead of '== None'
728750f	CVXOPTBackend.add_variables: Pass arguments to add_variable, correct default for lower_bound
b4a8ed7	Copy new CVXOPTBackend.add_variables tests to other backends
b0d89e4	GLPKBackend.add_variables: Set column names correctly
183ce25	PPLBackend.add_variable, add_variables: Don't silently ignore binary=True, integer=True
e47b608	CVXOPTBackend.add_linear_constraints: Add doctest, simplify code
b0179c2	Merge branch 't/20303/fixes_for_the_cvxopt_mip_backend' into t/20354/pplbackend__add_support_for_integer_variables
144a970	PPLBackend: Add support for integer variables

[mkoeppe]

Commit: 144a970

[mkoeppe]

Author: Matthias Koeppe

[dimpase]

comment:5

hmm:

cpdef int add_variable(self, lower_bound=0, upper_bound=None, binary=False, continuous=False, integer=False, obj=0, name=None) except -1:

should we really pick a default (continuous seems to be the most natural) rather than being it total denial?

[mkoeppe]

comment:6

Replying to @dimpase:

hmm:

cpdef int add_variable(self, lower_bound=0, upper_bound=None, binary=False, continuous=False, integer=False, obj=0, name=None) except -1:

should we really pick a default (continuous seems to be the most natural) rather than being it total denial?

This idiosyncratic interface (including the documentation that says that the default value of 'continuous' is True) matches that of the other backends.
I would probably express this by making all arglist-default None to indicate complicated defaulting semantics. But it's not the job of this ticket to make changes throughout all backends.

[dimpase]

comment:7

Replying to @mkoeppe:

Replying to @dimpase:

hmm:

cpdef int add_variable(self, lower_bound=0, upper_bound=None, binary=False, continuous=False, integer=False, obj=0, name=None) except -1:

should we really pick a default (continuous seems to be the most natural) rather than being it total denial?

This idiosyncratic interface (including the documentation that says that the default value of 'continuous' is True) matches that of the other backends.

but this is a line from your update, why do you set continuous=False?
Something you changed while developing and then forgot to revert?

[mkoeppe]

comment:8

No, continuous=True in the arglist cannot work
because then one cannot distinguish

• add_variable(..., binary=True) (which should create a binary variable)
• add_variable(..., binary=True, continuous=True) (which should raise an error).
  All backends implement the defaulting to continuous variables imperatively.

Previously it did not matter because the PPL backend was simply ignoring these parameters altogether.

(I've added a dicussion of this interface to ticket #20324.)

[dimpase]

comment:9

OK, I see. I think that the design with these logical switches is just wrong. There should be variable_type parameter, defaulting to continuous, with other possible values binary and integer.

[mkoeppe]

comment:10

Replying to @dimpase:

I think that the design with these logical switches is just wrong. There should be variable_type parameter, defaulting to continuous, with other possible values binary and integer.

I agree; at least it matches is_variable_continuous etc.; but see set_variable_type, which expresses the same thing using 1, 0, -1.
It's one of several idiosyncrasies of the backend interface.

[dimpase]

comment:11

How about introducing the variable_type parameter and deprecating the rest? (This is probably quite a bit of things to be changed then, in graphs/combinatorics code)
Similarly, let set_variable_type accept the same range of values, and deprecate 0,1,-1 ?

[mkoeppe]

comment:12

Replying to @dimpase:

How about introducing the variable_type parameter and deprecating the rest? (This is probably quite a bit of things to be changed then, in graphs/combinatorics code)
Similarly, let set_variable_type accept the same range of values, and deprecate 0,1,-1 ?

Sounds good; I have created ticket #20362 for this. Let's move further discussion of backend interface redesign there.

In the meantime, this ticket still needs review.

[dimpase]

comment:13

OK, looks good.

[dimpase]

Reviewer: Dima Pasechnik

[vbraun]

Changed branch from u/mkoeppe/pplbackend__add_support_for_integer_variables to 144a970