Account for transformation time in model.solve

[hbierlee]

Generally, we are not accounting for the pre-processing time (mostly transformation) in the given time_limit, nor in the reported solve time (i.e. slv.status().runtime will return the solve time without transformation; which means users should really time CPMpy themselves if they want to be accurate).

I'm not sure if we need to take some larger action here, but this simple change will at least subtract transformation time from the solve time when you solve directly from the model. There could be adverse effects, perhaps where now the user may not realize that these situations are now different:

model.solve(time_limit=10) # this gets less solving time than the solve call below

s = cp.solvers.CPM_ortools(model) # transformation happens here
s.solve(time_limit=10) # so this gets full 10 seconds

One way to make it more consistent would be to not immediately transform the model upon cp.solvers.CPM_ortools(model), instead waiting until the actual solve call.

[ElFosco]

Don't you have the risk that the time gets negative and the solver would throw an error? In the case that the transformations take too long.

[ThomSerg]

This is a tricky topic that we have discussed internally a couple of times which has a lot of caveats. So this should be more of a discussion topic.

Challenging parts / open questions:

• Your proposed change might break for some solvers when the time limit is already exceeded by the transformation portion (can all solvers handle negative time limits?)
• You now have two inconsistent behaviors for time_limit, since the transformation part will not actually adhere to it (i.e. aborting the transformation process)
• What with solveAll? It would then require the same new interpretation of time_limit. Will we keep track of all time lost in the overhead of callbacks on each found solution (we somewhat do this already)?

But it would definitely be something nice to have, since right now we hack something together for running in competitions, conceptually implementing a custom .solve() implementation.

[hbierlee]

I agree it should have been a discussion topic.

Let me know if it's already internally discussed a lot, and I'm not adding a lot of new ideas, but my replies to the above points would be:

• For first point from both Marco/Thomas, the solvers are tested to raise a ValueError for negative time limits
• Related, I agree then that the behavior is indeed different because we raise a ValueError for negative time limits rather than returning False and set status to UNKNOWN as we do for solve. If you just see CPMpy as a big solver, then immediately returning UNKNOWN status would make more sense than the ValueError, and make things consistent if you run out of time during TF.
  • The fact that we don't check the time limit during TF is in some way irrelevant (essentially we do check, just once, at start of solve after TF). It would be good to add checks in-between constraints though, ensuring we stop (potentially much) closer to the time-out. I don't think there is any significant overhead to this.
• For solveAll yes, I guess it should already take the callback time into account. Indeed, we somewhat do that, as I see that solver_interface does take the callback time (including the TF of the blocking constraint), while Exact does not. That could be a separate issue, as just timing everything in-between the callback would seem best. This change just focuses on also taking the initial TF time into account in solve/solveAll.

[OrestisLomis]

For point 2 I would say that after the transformations are done you can indeed simply check the time and then without even calling solve return UNKNOWN status. The ValueError seems to be more of a error that should be thrown to the user when they themselves give a negative timeout, not something we should use to stop the solving.

[ThomSerg]

For those interested, the current way to take transformation time into account is by doing the following:

Instead of

model.solve(solver="ortools", time_limit=60) # example of time limit of 60s, only on the solve part (not taking transformation into account)

You can do

start = time.time()
s = cp.SolverLookup().get("ortools", model) # This will only do the transformation to the solver backend, without solving
end = time.time()
transformation_time = end - start

s.solve(time_limit=60-transformation_time) # Now solve without re-transforming, run with remaining time

Offcourse this still doesn't take all overhead into account, so more can be subtracted if needed (e.g. a small fixed buffer).

[tias]

I'm in favor of changing the model.solve() in this respect yes; when you call a function that has a time_limit argument, you expect the function itself to respect its argument (I neglected the potential time difference when making the function).

It also exposes another aspect of that neglection: that SolverStatus.runtime is only the solver time, not the actual runtime.

So perhaps we need a bit of a more throurough change to capture all things discussed here, e.g. maybe something like:

• Add a 'SolverStatus.solvertime' property?
• Have each CPMpy solver class fill in solvertime from the solver object, and runtime from its own time.time() in solve()
• Have model.solve() overwrite runtime from its own timer
• Have model.solve() create its own Status with unknown if time limit reached before solve (we can for now neglect that it might go over the time limit when creating itself... doing that properly would require multi-threading or time_limit on add, which goes a bit too far imho)

I would keep a similar exercise on SolveAll as a separate issue/PR