Add incremental flag for faster one-shot Exact and SCIP solves#1020
Add incremental flag for faster one-shot Exact and SCIP solves#1020tias wants to merge 1 commit into
Conversation
Model.solve() can use faster non-incremental solver paths when the solver object is discarded after a single call, while direct solver reuse keeps incremental=True by default.
IgnaceBleukx
left a comment
IgnaceBleukx
left a comment
There was a problem hiding this comment.
Seeing this, I'm not sure if we should add it to the constructor args? Maybe better to add a flag to the .solve function instead?
|
|
||
|
|
||
| def __init__(self, cpm_model=None, subsolver=None, **kwargs): | ||
| def __init__(self, cpm_model=None, subsolver=None, incremental=True, **kwargs): |
There was a problem hiding this comment.
I feel it's too specific; we have a call_from_model flag that we pass to solveAll, we could re-use something similar here? There it is used to determine whether we want to print a warning or not I believe, here we can use it so select the solve func of exact.
| my_status, obj_val = self.xct_solver.toOptimum(timeout=timeout) | ||
| obj_val = None | ||
| if self.incremental: | ||
| my_status, obj_val = self.xct_solver.toOptimum(timeout=timeout) |
There was a problem hiding this comment.
I would not trust the objective val computed by exact in this special path? in the previous version, we would always just recompute it from the expresssion object, which I think is the right way
| # and the user can now change the model as they will. The downside is that potentially | ||
| # useful information for speeding up the next optimisation call is thrown out. | ||
| self.scip_model.freeTransform() | ||
| if self.incremental: |
There was a problem hiding this comment.
We could just always call freeTransform, no? If we do smt incremental then we should definetly, if its not incremental because we call from model, then it doesn't hurt either? Or can this call be expensive?
| if kwargs and solver is None: | ||
| raise NotSupportedError("Specify the solver when using kwargs, since they are solver-specific!") | ||
|
|
||
| if isinstance(solver, str): |
There was a problem hiding this comment.
If we decide to go the call_from_model flag-way then this is not needed
2 participants
Closes #968.
For one-shot solves via
Model.solve(), Exact and SCIP were doing extra work meant for reusing the solver object. Exact always went throughtoOptimum(); SCIP always calledfreeTransform()afteroptimize(). Both are useful when you keep the solver and solve again, butModel.solve()throws the solver away anyway.This adds an
incrementalflag on both interfaces (defaultTrue).Model.solve()passesincremental=Falsefor exact and scip. Exact then usesrunFull; SCIP skipsfreeTransform().Left
Model.solveAll()alone — SCIP's generic enumeration callssolve()in a loop and adds nogoods, so it still needs the incremental path.Added a couple of tests that check
Model.solve()gives the same result asSolverLookup.get(..., incremental=True).