scip interface (first version)

[tias]

with help of Mark Turner from SCIP.

Now... turns out that @IgnaceBleukx already worked on a SCIP interface too, about a year ago (e.g. SCIP branch

So maybe Ignace can now make a best of both worlds? : )
(current branch is up to date with current template, but the add part is minimal, e.g. reified linear is not properly tested/supported at the least).

[IgnaceBleukx]

Yes, I would be happy to merge both versions of the interface!

[IgnaceBleukx]

Updated the interface somewhat, it seems SCIP does not have the fancy Min/Max/Abs constraints built-in in its API : /

Added the SOS1 and cardinality constraints to the interface too, these constraints are created when decomposing alldiff for example so definately usefull to have them imho. We should also add them to the Gurobi interface

SolveAll is in a weird state for PySCIPOpt: SCIP does allow for retrieving all feasible solutions, but the Python interface does not seem to allow for collecting the solutions which are found... Added the issues to track in the code so we can add the specialized implementation when these are resolved.

All tests are passing so ready for review I think.

[Wout4]

I get an error for optimization problems:

\cpmpy\solvers\scip.py", line 154, in solve
    self.objective_value_ = scip_objective.getObjVal()
AttributeError: 'pyscipopt.scip.Expr' object has no attribute 'getObjVal'

[Opt-Mucca]

I get an error for optimization problems:

\cpmpy\solvers\scip.py", line 154, in solve
    self.objective_value_ = scip_objective.getObjVal()
AttributeError: 'pyscipopt.scip.Expr' object has no attribute 'getObjVal'

This should now be addressed in one of my points above

[Opt-Mucca]

@IgnaceBleukx @tias I haven't run anything, and truthfully haven't tried to understand the larger code base, but I've gone through and given comments that I think should make the interface work.
For a final point: While min / max expressions probably won't work, SCIP can handle the absolute value expressions fairly easily. I'm just not sure where to add them in the current code without some guidance.

[hbierlee]

SCIP PR ready for review with some notable update:

• Came across some additional sensible cases for solver_interface tests ; these are on a separate branch
• simplified XOR handling, which is currently untested in CI, but will be tested by Xor simplify pr. I confirmed it worked manually.

Most notably, I ran pytest-cov to get a coverage on what lines of codes are actually touched by the CI. It's very instructive. If a line of code is not run by CI, either the test for it is missing, or more likely, it is dead code. This left to the following simplifications:

• removed: SCIP supports abs at root, but not reified, so there was code to ad-hoc transform it. This code never fired, since reified abs is rewritten (in flatten I think).
• removed: sub support ; never fired because linearize always rewrites this (i.e. it does not check supported for it). I didn't want to add support for it in this PR, because we should think about whether it is even useful for linears?
• removed: != support: there was code to to ad-hoc transform this (using the native or handling of SCIP, saving a binary variable), but similarly as sub, linearize does not check supported for it. Then I added it as supported but this meant we now have to support b -> ... != .. as well (and I cannot indicate, support != only when non-reified, at least, that will get very messy). Handling reified != is a little complicated to make work with that ad-hoc transform. The solution IMO is to add proper support for or if SCIP supports it (and thus NOT transform != but rewrite it into (.. > ..) | (.. < ..)). This all felt very out of scope so I think for the SCIP interface, we should just do != similar to other ILP solvers.

Recommend checking out the reports below for an older code cov (htmlcov-bak/scip.html) report (from running pytest --solver=scip) which revealed this issues, and the new one which shows only "unimportant" code is not covered (although missing coverage for both DirectConstraint and FEASIBLE status when a proper subset of all solutions have been enumearted (e.g. within timeout) may still be worth a look).

coverages.zip

[tias]

very nice and clean, made some minor tweaks and minor comments

(also a less minor comment that is just triggered by the review, need not be for the PR)

[tias]

do we do this function the same elsewhere?

its not written with 'fast path' in mind... an alterantive would be

solver_var = self._varmap.get(cpm_var, None)

if solver_var = None:
  if is_num...
  elif is _BoolVarImpl:
    if NegBool: raise
    else:
       solver_var = ...
  elif _IntVarImple:
  self._varmap[cpm_var] = solver_var

return solver_var

if other solvers do similar like we do here, we should fix this in a separate PR

[hbierlee]

@tias fixed comments (and I can't request your review since you started the PR)

[tias]

I approve very much, but can't formally approve because I started the PR indeed : )

But its your earlier review that is blocking it, so please approve if you approve and then merge ; )