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.

Sign up for GitHub

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

Jump to bottom

DNMY: MOI #125

Merged
merged 43 commits into from
Jul 24, 2018
Merged

DNMY: MOI #125

merged 43 commits into from
Jul 24, 2018

Conversation

odow
Copy link
Member

@odow odow commented Apr 27, 2018 •
edited

Blockers

This PR

I've copied the code from https://github.com/JuliaOpt/MathOptInterfaceGurobi.jl

MOI tests currently fail.

image

@mlubin
Copy link
Member

mlubin commented Apr 27, 2018

What about tagging a new release that covers 8.0 before merging this?

@mlubin
Copy link
Member

mlubin commented Apr 27, 2018

Also dropping julia 0.5...

@odow
Copy link
Member Author

odow commented Apr 27, 2018

Yes I was about to ask that. Shall we leave Julia 0.5, tag as v0.3.4, then drop julia 0.5, merge this and tag as v0.4.0?

@mlubin
Copy link
Member

mlubin commented Apr 27, 2018

AFAIK we can't even tag a new version that claims to support 0.5. So drop 0.5 and tag Gurobi.jl 0.4.

@odow
Copy link
Member Author

odow commented Apr 27, 2018

Tests are green!

@blegat
Copy link
Member

blegat commented Apr 27, 2018

We decided on the name MOIWrapper.jl (hence also MPBWrapper.jl) for both the file in src and test. Do you agree on the naming or do you have a reason to use MathOptInterface/mathoptinterface_wrapper.jl instead ?

@odow
Copy link
Member Author

odow commented Apr 27, 2018

Renamed to MOIWrapper.jl in src and test. However I've left the MPB tests in the folder since they are in lots of different files.

@odow odow changed the title WIP: Add code from MathOptInterfaceGurobi and reorganise DNMY: Add code from MathOptInterfaceGurobi and reorganise Apr 28, 2018
@odow odow changed the title DNMY: Add code from MathOptInterfaceGurobi and reorganise DNMY: MOI Apr 28, 2018
@rdeits
Copy link
Collaborator

rdeits commented May 10, 2018

I'm getting the same issue here as in jump-dev/GLPK.jl#53 (comment) : I can set @objective model Min x + 1 but not @objective model Min x due to Unsupported attribute MathOptInterface.ObjectiveFunction{MathOptInterface.SingleVariable}(). Or maybe I'm misunderstanding how the new MOI should be used?

@odow odow mentioned this pull request May 12, 2018
Merged
odow
odow commented May 17, 2018
View reviewed changes
test/MOIWrapper.jl Outdated
nothing
end

MOI.set!(m, Gurobi.CallbackFunction(), callback_function)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joaquimg @mlubin: callbacks in MOI.

odow
odow commented Jun 6, 2018
View reviewed changes
test/MOIWrapper.jl
# (0,0) +---+---+ (2,0)
TOL = 1e-6 # Allow for some impreciseness in the solution
if y_val - x_val > 1 + TOL
Gurobi.cblazy!(cb_data, m,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mlubin @joaquimg @blegat

Lazy constraints through MOI

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oarsome!

odow
odow commented Jun 6, 2018
View reviewed changes
test/MOIWrapper.jl Outdated
if cb_where == Gurobi.CB_MIPSOL
sol = [0.0, 0.0]
Gurobi.cbget_mipsol_sol(cb_data, cb_where, sol)
x_val, y_val = sol
Copy link
Member Author

@odow odow Jun 6, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing is it's not clear how to inject the MIP solution back into MOI-land so that it can be queried via VariablePrimal

@odow odow mentioned this pull request Jun 24, 2018
Merged
@rdeits rdeits mentioned this pull request Jun 26, 2018
Closed
@odow
Copy link
Member Author

odow commented Jun 28, 2018

This is now rebased on to master.

See jump-dev/MathOptInterface.jl#404, JuliaOpt/LinQuadOptInterface.jl#30, JuliaOpt/LinQuadOptInterface.jl#31 for the commented tests

@rdeits rdeits mentioned this pull request Jul 18, 2018
Merged
@tkoolen
Copy link
Contributor

tkoolen commented Jul 18, 2018

This (or LQOI) has an issue with duplicate terms in the affine part of the objective function:

using MathOptInterface
using Gurobi
using Compat.Test

const MOI = MathOptInterface

atol = 1e-4
rtol = 1e-8

# Minimize (x - 1.5)^2 = x^2 - 3 * x + 2.25
model = GurobiOptimizer(OutputFlag=0)
v = MOI.addvariables!(model, 1)

# this works:
# obj = MOI.ScalarQuadraticFunction([MOI.ScalarAffineTerm(-3.0, v[1])], MOI.ScalarQuadraticTerm.(2.0, v, v), 2.25)

# this doesn't:
obj = MOI.ScalarQuadraticFunction([MOI.ScalarAffineTerm(-1.5, v[1]), MOI.ScalarAffineTerm(-1.5, v[1])], MOI.ScalarQuadraticTerm.(2.0, v, v), 2.25)

MOI.set!(model, MOI.ObjectiveFunction{MOI.ScalarQuadraticFunction{Float64}}(), obj)
MOI.set!(model, MOI.ObjectiveSense(), MOI.MinSense)
MOI.optimize!(model)
@test MOI.get(model, MOI.VariablePrimal(), v) ≈ [1.5] atol=atol rtol=rtol

(This is the reason for tkoolen/Parametron.jl#31 (comment)).

tkoolen
tkoolen reviewed Jul 18, 2018
View reviewed changes
src/MOIWrapper.jl
nvars = num_vars(instance.inner)
obj = zeros(Float64, nvars)
for (col, coef) in zip(columns, coefficients)
obj[col] = coef
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a += to support duplicate terms.

tkoolen added a commit to tkoolen/MathOptInterface.jl that referenced this pull request Jul 18, 2018
@tkoolen
Add duplicate terms to linear part of objective in qp2test
a9db7cd 
To catch
jump-dev/Gurobi.jl#125 (comment),
jump-dev/Xpress.jl#22,
jump-dev/GLPK.jl#57.
@tkoolen tkoolen mentioned this pull request Jul 18, 2018
Closed
@rdeits rdeits mentioned this pull request Jul 24, 2018
Merged
@odow
Copy link
Member Author

odow commented Jul 24, 2018

Since MOI and LQOI are settling down, I'm going to merge this. We can fix up any outstanding issues in separate PRs.

@odow odow merged commit 1b454c8 into master Jul 24, 2018
@rdeits
Copy link
Collaborator

rdeits commented Jul 24, 2018

@odow did you resolve the issue with duplicate terms? It's fine if not, but we should open an issue to be sure it doesn't get forgotten.

@odow odow deleted the odow/moi branch July 24, 2018 23:34
This was referenced Jul 24, 2018
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkoolen tkoolen tkoolen left review comments

@joaquimg joaquimg joaquimg approved these changes

@blegat blegat blegat approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@odow @mlubin @blegat @rdeits @tkoolen @joaquimg