SolveInParallel with Generators

[AlexandreAmice]

This change is 

[AlexandreAmice]

+(status: do not merge)
cc @cohnt

Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", missing label for release notes (waiting on @AlexandreAmice)

[AlexandreAmice]

+@jwnimmer-tri for feature review? Thanks!
-(status: do not merge)

Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

---

geometry/optimization/convex_set.cc line 130 at r2 (raw file):

  };

  // This worker lambda maps the index i to a dimension and direction to check

The changes here were made to show a use case for the new code and how it avoids duplication of SolveInParallel type logic in other parts of the code base.

---

solvers/solve.h line 109 at r2 (raw file):

 * size as progs.
 *
 * @throws std::exception if any of the progs are nullptr.

The new generator form of the code accepts progs which are nullptr so that one can exit early. Should we allow this in the pre-allocated vector form now?

@cohnt for feedback

---

geometry/optimization/graph_of_convex_sets.cc line 861 at r2 (raw file):

    // Check if edge e = (u,v) could be on a path from start to goal.
    std::vector<const MathematicalProgram*> prog_ptrs(progs.size());

The new templated version makes code having to copy raw pointers of unique pointers unnecessary.

---

geometry/optimization/graph_of_convex_sets.cc line 829 at r2 (raw file):

  int nE = edge_id_list.size();

  // TODO(cohnt): Rewrite using generators to avoid batching.

This illustrates another part of the code base which would benefit from the new code.

[jwnimmer-tri]

Thanks, I will take a closer look over the next few days.

Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

---

a discussion (no related file):
My initial impression from a first skim is that having multiple independent callback functions is big hassle (program vs solver_id vs solver_options). Did you consider a design where there is only a single callback function, which operates on a dataclass-like struct? My gut feeling is that it would be much cleaner, though I'd need to pencil it out to be sure.

[AlexandreAmice]

Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

My initial impression from a first skim is that having multiple independent callback functions is big hassle (program vs solver_id vs solver_options). Did you consider a design where there is only a single callback function, which operates on a dataclass-like struct? My gut feeling is that it would be much cleaner, though I'd need to pencil it out to be sure.

So I thought about putting the callbacks into a struct like

struct SolveInParallelCallbacks {
const std::function<T(int64_t, int64_t)>& prog_generator
const std::function<std::optional<Eigen::VectorXd>(int64_t, int64_t)>&
        initial_guesses_generator,
    const std::function<std::optional<SolverOptions>(int64_t, int64_t)>&
        solver_options_generator,
    const std::function<std::optional<SolverId>(int64_t, int64_t)>&
        solver_ids_generator
}

but then I think overloading with the variants where a single solver_option and solver_id are passed becomes a bit messier.

If you have a different design in mind I would welcome the feedback.

[jwnimmer-tri]

Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

---

a discussion (no related file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

So I thought about putting the callbacks into a struct like

struct SolveInParallelCallbacks {
const std::function<T(int64_t, int64_t)>& prog_generator
const std::function<std::optional<Eigen::VectorXd>(int64_t, int64_t)>&
        initial_guesses_generator,
    const std::function<std::optional<SolverOptions>(int64_t, int64_t)>&
        solver_options_generator,
    const std::function<std::optional<SolverId>(int64_t, int64_t)>&
        solver_ids_generator
}

but then I think overloading with the variants where a single solver_option and solver_id are passed becomes a bit messier.

If you have a different design in mind I would welcome the feedback.

I think I wasn't clear. I was thinking of a per-solve helper struct as the callback argument for each i'th program instance, something like:

struct IthSolveArgs {
   shared_ptr<const MathematicalProgram> prog;
   optional<Eigen::VectorXd> initial_guess;
   optional<SolverId> solver_id;
   shared_ptr<const SolverOptions> solver_options;
};

and then the "generator" callback is something like

std::function<bool(int i, IthSolveArgs* args)> make_ith_program

where the callback mutates the args with the i'th details.

I added the bool return value with the idiom of "return keep_going" so if the callback decides to cancel the solving, it can return false for that. (In python we would map None to true, to keep going by default.)

[AlexandreAmice]

Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I think I wasn't clear. I was thinking of a per-solve helper struct as the callback argument for each i'th program instance, something like:

struct IthSolveArgs {
   shared_ptr<const MathematicalProgram> prog;
   optional<Eigen::VectorXd> initial_guess;
   optional<SolverId> solver_id;
   shared_ptr<const SolverOptions> solver_options;
};

and then the "generator" callback is something like

std::function<bool(int i, IthSolveArgs* args)> make_ith_program

where the callback mutates the args with the i'th details.

I added the bool return value with the idiom of "return keep_going" so if the callback decides to cancel the solving, it can return false for that. (In python we would map None to true, to keep going by default.)

I see. I'll try to pencil that out and might open a separate PR with that design so we can compare.

My intention was to avoid binding this in Python as I am worried about concurrently accessing Python states in the callbacks from C++.