Skip to content
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

Better error for unsupported constraint when the optimizer is provided at optimize! #1504

Closed
blegat opened this issue Sep 29, 2018 · 7 comments
Assignees
Milestone

Comments

@blegat
Copy link
Member

blegat commented Sep 29, 2018

When the optimizer is given in the Model constructor, unsupported added constraint are either bridged or throw an helpful error:

https://github.com/JuliaOpt/JuMP.jl/blob/500945cac4949dfd9e30cb0ca68450f19f4ff34d/src/constraints.jl#L202

If it is given in the optimize! call, a cryptic error is thrown instead.
See jump-dev/SCS.jl#112

@mlubin
Copy link
Member

mlubin commented Oct 25, 2018

Why don't bridges work when the optimizer is provided in optimize!?

@blegat
Copy link
Member Author

blegat commented Oct 25, 2018

Because the bridges are applied on top of the CachingOptimizer. So when constraints are added and no optimizer is set yet, the caching optimizer says that everything is supported and nothing is bridged.
Then when the optimizer is added, the caching optimizer copies its model to the optimizer "below" the bridges and the bridges cannot act.
The reason bridges are "above" the caching optimizer is that bridges currently need the underlying optimizer to support add_constraint and add_variable as it bridges constraints on-the-fly.

One solution is to make bridging also work through the Allocate-Load API in which case, bridges will also work on-the-fly for solvers not supported add_constraint and add_variable.
This is what is proposed in light bridges.
That will allow to apply bridges below the caching optimizer and hence make it work when the optimizer is set in optimize!.
The issue is that it will makes bridges unusable for optimizer that are implementing copy_to "by hand" without using allocate_load or default_copy.
However, I don't think that saying "you can implement copy_to by hand but that will make your optimizer incompatible with bridges" is a big issue. For instance OSQP currently does it by hand but it is rather simple to convert it to using allocate-load.

@mlubin
Copy link
Member

mlubin commented Oct 26, 2018

CachingOptimizer was intended to be the JuMP moi_backend and I didn't fully understand the implications of putting a layer on top of it. This has to be fixed for 0.19.

The allocate-load API is not nearly as well documented as the rest of MOI and intrinsically more complicated which together make it about 5x harder to understand than the plain copy_to API. I don't want to force solver wrappers to use it. Can't we throw another CachingOptimizer in the mix if needed to give the bridges an add_constraint interface to copy-only solvers? It makes sense that for copy-only solvers we might need 3 copies of the problem: 1. original, 2. transformed, and 3. the solver's internal version.

@blegat
Copy link
Member Author

blegat commented Oct 26, 2018

Can't we throw another CachingOptimizer in the mix if needed to give the bridges an add_constraint interface to copy-only solvers?

Yes, we could do that but it would be nice to have a way to check whether the optimizer supports add_constraint and add_variable in which case we don't add a second cache.

@mlubin
Copy link
Member

mlubin commented Oct 26, 2018

Yes, we could do that but it would be nice to have a way to check whether the optimizer supports add_constraint and add_variable in which case we don't add a second cache.

That seems like a reasonable feature to add. Maybe a trait that the optimizer can implement?

@blegat
Copy link
Member Author

blegat commented Oct 26, 2018

Yes, we could even (optionally?) have no caching optimizer at all if the solver implement the trait.
But that would require the solver to also support names, getters, ... so we may need 2 traits.

@blegat
Copy link
Member Author

blegat commented Nov 10, 2018

Will be fixable once jump-dev/MathOptInterface.jl#561 is merged and MOI is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants