scikit-decide icon indicating copy to clipboard operation
scikit-decide copied to clipboard

Solver API inconsistency

Open neo-alex opened this issue 3 years ago • 1 comments

🐛 Bug

Some solvers in the hub require domain_factory in their constructor, although it is not enforced by the API (usually this parameter comes from the solve_with method on a domain).

Expected behavior

domain_factory should ideally be removed from every solver constructor ; if this is not possible for some reason (e.g. proper initialisation of C++ solvers...), then try to align on an API that could work consistently across all solvers & update the doc.

neo-alex avatar Nov 19 '21 09:11 neo-alex

Here are some (hopefully) useful information to help solve the issue. The domain_factory is passed to the solver constructor for the sake of parallel domains, be the underlying solver implemented in Python or in C++. The process is the following:

  1. Create the solver instance
  2. If parallel is passed to the solver's constructor, create as many domains as the number of CPUs in separate parallel processes, using domain_factory
  3. Solve the domain, potentially many times from different states, which can happen during the rollout with incomplete or partial solvers. Note that it would be inefficient to create the domains each time the solving method of the solver is called, since it requires to open pipes or create shared memory spaces. This is why the parallel domains must be created before calling the solving method.
  4. Close the solver by destroying the parallel domains, and by closing the parallel process pipes or the shared memory spaces

The need for creating the parallel domain instances before calling the solving method many times, and for closing them afterwards, calls for using the with context manager with a solver instance.

One way to not pass the domain_factory to the solver constructor, would consist in creating the parallel domain instances on the first call to the solving method of the solver, i.e. opportunistically creating them if they don't already exist.

fteicht avatar Nov 19 '21 16:11 fteicht

Solved by #363.

nhuet avatar May 21 '24 11:05 nhuet