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

Questions implementing a wrapper #71

Closed
3 tasks done
odow opened this issue Jul 21, 2017 · 19 comments
Closed
3 tasks done

Questions implementing a wrapper #71

odow opened this issue Jul 21, 2017 · 19 comments

Comments

@odow
Copy link
Member

odow commented Jul 21, 2017

I'm going to use this issue as a collection place for a range of questions I have implementing the CPLEX solver wrapper:

On the purpose of low-level interface

  • should we expose the full (potentially ad-hoc, complicated) API, or a simplified one to satisfy MOI requirements. Don't remove existing functionality

On variable bounds

  • is addconstraint!(m, v::VectorOfVariables, set::LessThan{Float64}) a valid constraint? oO
  • should addconstraint!(m, v::VectorOfVariables, set::Vector{Union{LessThan{Float64}, GreaterThan{Float64}, EqualsTo{Float64}, Interval{Float64}}}) be a valid constraint so users can set bounds all at once? No. Overload the broadcast operator performance hints #63 (comment) performance hints #63 (comment)
@joaquimg
Copy link
Member

Maybe with plural: addconstraints!(m, v::VectorOfVariables, set::Vector{Union{LessThan{Float64}, GreaterThan{Float64}, EqualsTo{Float64}, Interval{Float64}}}) this could return vector of references...

@mlubin
Copy link
Member

mlubin commented Jul 21, 2017

is addconstraint!(m, v::VectorOfVariables, set::LessThan{Float64}) a valid constraint?

We can't prevent a solver from recognizing the constraint, but no (#64). VectorOfVariables is a vector-valued function and LessThan is a scalar-input set.

should addconstraint!(m, v::VectorOfVariables, set::Vector{Union{LessThan{Float64}, GreaterThan{Float64}, EqualsTo{Float64}, Interval{Float64}}}) be a valid constraint so users can set bounds all at once?

It's one function per set in MOI. VectorOfVariables is one function, so it doesn't make sense to provide multiple sets there. We could define addconstraints!(m, ::Vector{F}, ::Vector{S}) to return a vector of ConstraintReference{F,S} for concrete F and S. I don't see a reasonable (type-stable) way to do that when S is a union type .

should we expose the full (potentially ad-hoc, complicated) API, or a simplified one to satisfy MOI requirements.

Up to the discretion of whoever is writing the MOI wrapper. Keep in mind that eventually there will be a desire for a standalone and feature complete CPLEX wrapper (#22). It's not our job to do that, but removing existing wrappers for C functions that aren't used in MOI seems unnecessary. It's fine with me to remove higher-level helper functions that aren't needed for MOI.

mlubin added a commit that referenced this issue Jul 24, 2017
mlubin added a commit that referenced this issue Jul 24, 2017
mlubin added a commit that referenced this issue Jul 25, 2017
@IssamT
Copy link
Contributor

IssamT commented Jul 31, 2017

SolverInstance(solver::AbstractSolver) is supposed to create the new model to be solved. Is there a way to tell a solver who is able to solve different types of models , requiring different setups, which type of model you want to solve? (for example what type of constraints will be added in this model)

@blegat
Copy link
Member

blegat commented Jul 31, 2017

Not to my knowledge. You could store the constraints in a MathOptInterfaceUtilities's Instance until you figure out which setup you need and then you set up the solver, load the constraints and throw the instance away.

@IssamT
Copy link
Contributor

IssamT commented Jul 31, 2017

To keep MOI generic the type of the model, in my opinion, should be just an optional argument. For some model types like LPs where there is no variable type and only linear constraints, it could be nice for the solver to know it.

CLP for example has:

LinearQuadraticModel(s::ClpSolver) = ClpMathProgModel(;s.options...)
ConicModel(s::ClpSolver) = LPQPtoConicBridge(LinearQuadraticModel(s))

For Cplex I think that if you declare the problem as LP you can't add integer variables afterwards.

MOIU despite adding an intermediary layer doesn't solve the issue because it doesn't tell you anything about what the user is planning to add after the optimize!

EDIT:
My example with CLP is not appropriate since in this case the only difference is probably due to MPB interface as @blegat mentioned.

@IssamT
Copy link
Contributor

IssamT commented Jul 31, 2017

This may be a partial answer to my question, for solvers which rely on MOIU

https://github.com/JuliaOpt/MathOptInterfaceUtilities.jl/blob/master/src/instance.jl#L229

@joaquimg
Copy link
Member

joaquimg commented Jul 31, 2017

MOIU despite adding an intermediary layer doesn't solve the issue because it doesn't tell you anything about what the user is planning to add after the optimize!

For stuff being added after optimize! we should have other attributes like: SupportsAddVariableAfterSolve.

@joaquimg
Copy link
Member

CLP has multiple solvers?

@IssamT
Copy link
Contributor

IssamT commented Jul 31, 2017

One solver but two possible models (solver instances)

@IssamT
Copy link
Contributor

IssamT commented Jul 31, 2017

We can maybe use a SolverInstanceAttribute for this.

cplex example of the function to be called by the attribute setter:
https://www.ibm.com/support/knowledgecenter/en/SSSA5P_12.7.1/ilog.odms.cplex.help/refcallablelibrary/cpxapi/chgprobtype.html

And if a solver needs the problem type before creating the instance we can create the instance only on julia side and delay calling the true constructor until the attribute has been set.

@mlubin
Copy link
Member

mlubin commented Jul 31, 2017

If we force everyone to specify a "problem type" when calling SolverInstance then we also have to put this concept into JuMP, otherwise people will be confused when they get error messages about not being able to add certain types of constraints while the JuMP model is attached to the solver instance. I don't like it. The alternatives are:

  • Use two different solver objects like GLPKSolverMIP() and GLPKSolverLP() or a parameter like CplexSolver(problemtype = :LP).
  • Abstract away the difference in the MOI wrapper as @blegat suggested.

@mlubin
Copy link
Member

mlubin commented Jul 31, 2017

We can maybe use a SolverInstanceAttribute for this.

I don't have a good understanding of what CPLEX's problem type actually means. Using a SolverInstanceAttribute is possible but on a first glance it seems like it would have to be something solver specific because other solvers have either different concepts or no concepts at all of a problem type.

@odow
Copy link
Member Author

odow commented Jul 31, 2017

CPLEX's problem type is used to dispatch on different optimize calls. For instance, if the problem type is MILP, a call to CPXlpopt will error. It's pretty annoying as you have to keep track of what type of problem you're solving, rather than letting CPLEX figure it out.

@joaquimg
Copy link
Member

joaquimg commented Sep 7, 2017

@odow I was looking at the latest CPLEX MOI.

It seems that there is a lot of work around exposing all the MOI flexibility which differs from MPB that was much simpler.

You have written the code so that it is explicit when you use a cplex function. I listed most of them here https://gist.github.com/joaquimg/e793e5a0677d77b0e190df556f58e178

Many of them map DIRECTLY into some functions of Xpress and Gurobi. And some other need simple adaptations.

What do you think about having a intermediate package (SomeCleverNameOptInterface) to make it easier to wrap the 3 of them in MOI? just like SOI does for its solvers... Specific corner cases could be handled by the wrappers (CPLEXMathOptInterface...)

@joaquimg
Copy link
Member

joaquimg commented Sep 7, 2017

If I copy what you have done and change a few functions I get 90% of the functionality...

@odow
Copy link
Member Author

odow commented Sep 7, 2017

I'm a bit busy writing up bits of my thesis for a few weeks., but this seems sensible.

@joaquimg
Copy link
Member

joaquimg commented Sep 8, 2017

I started the experiment: https://github.com/joaquimg/LinQuadOptInterface.jl
I am terrible at naming things, so please give me better ideas.

The Xpress version is in LQOI branch in Xpress.jl and the CPLEX version consists of simply replacing "lqs" by "cpx"
It looks good, Xpress is passing almost everything it was passing before in less than 400 lines. This way gurobi might be very easy. Plus performance optimizations would go for the 3 of them at the same time.

@blegat
Copy link
Member

blegat commented Sep 8, 2017

I am glad to see that much of the code can be shared between the LQ solvers ! The name LinQuadOptInterface seems fine to me, it is consistent with SOI. I wonder if GLPK and Clp can use LQOI.

@odow
Copy link
Member Author

odow commented Nov 18, 2017

This discussion seems mostly resolved.

@odow odow closed this as completed Nov 18, 2017
odow pushed a commit that referenced this issue Sep 23, 2019
* Update LP format to sanitise variable names.

Code ported from LPWriter.jl, changed regex API so it runs in Julia 1.1.
Other changes: constraints must have a "=" sign, say CPLEX and lp_solve; add an End keyword, says CPLEX.

* Add a test case for the name sanitisation.

* Adding more test cases, ported from LPWriter.

* @odow's nit-picking :).

* Implement a name cache.

Based on https://stackoverflow.com/a/33783534

* Rework the cache mechanism to pass it as an argument.

* Handle created duplicate names in sanitisation.

* Ensure the generated names are never too long.
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

5 participants