-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add solver_interface
and solver_option
as optional members of ConvexSet
#21251
Comments
cc @hongkai-dai, @cohnt , @RussTedrake, and @sadraddini as all of you may have thoughts. |
+1 from me on this. I can think of multiple cases where I would leverage this! |
A draft PR is available at #21253 to discuss the design. An example of changing the method in |
It is indeed useful. However, I feel that such an API is not right, as convex set object instances will carry extra information. Many convex set methods do not rely on optimization. I still prefer a more direct API such that for each method that solves a mathematical program, the user can specify the solver. Also, the user may want to use different solvers for different methods with the same |
For the design with the solver as a shared_ptr member field of ConvexSet, it also raises difficult questions about thread safety. Previously, if I took a ConvexSet object and handed it off from one thread to another, the other thread could do as it pleases. If the solver is a shared_ptr member field, now we have possibly many sets on different threads all trying to call the same solver instance, which is not threadsafe. On the other hand, the solver options can easily be either const or copied around, so do not raise questions about thread safety. Still, aesthetically I agree with @sadraddini that having the options come in as arguments to one specific operation at a time is likely to be more sensible. If we could see some examples of how this would be used, that might sway the API design one way or another. |
Thank you all for commenting on the API, as I don't necessarily think my way is the best way. My high level goals with the current design was to avoid having to specify the solver instance every time a method is called. The other awkwardness I see with having the The simplest example of this is the Thoughts on how to resolve this? That is a very good point about the shared pointer. @jwnimmer-tri I suppose |
Many of the method in
ConvexSet
rely on solving an optimization program to return an answer. Examples includeIsEmpty()
,PointInSet()
,MaybeGetFeasiblePoint()
,IsEmpty()
, etc.Sometimes, it would be good for the user to be able to both specify the solver used and potentially some options for that solver especially for debugging purposes.
This proposal is similar in scope to #20681 but much narrower.
The text was updated successfully, but these errors were encountered: