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

DNMY: MOI #53

Merged
merged 36 commits into from
Jun 28, 2018
Merged

DNMY: MOI #53

merged 36 commits into from
Jun 28, 2018

Conversation

@odow odow changed the title WIP: Add MOI DNMY: MOI Apr 28, 2018
@mlubin
Copy link
Member

mlubin commented May 1, 2018

@carlobaldassi, note this is reverting our decision from 2013 to split off the JuMP-related code into a separate package. Things have changed a bit since then :).
Our current preference is to host the MathOptInterface code into the solver-wrappers for ease of maintainability and testing, unless there's a good reason not to.

@odow I don't see a need to move the code from GLPKMathProgInterface into here. It's just extra churn for code that soon won't be relevant.

@odow
Copy link
Member Author

odow commented May 7, 2018

@mlubin Do you want to tag a release before this PR gets merged (or give me commit access :) )?

@mlubin
Copy link
Member

mlubin commented May 8, 2018

@odow, all yours :)

@rdeits
Copy link
Collaborator

rdeits commented May 10, 2018

It looks like there's an attribute missing from the GLPKOptimizerLP(). Using this branch (and JuMP and MOI master), I can set up a simple LP:

m = Model(optimizer=GLPKOptimizerLP())
@variable m x >= 0
@objective m Min x
JuMP.optimize(m)

which fails with:

Failed to copy model into optimizer: Unsupported attribute MathOptInterface.ObjectiveFunction{MathOptInterface.SingleVariable}()

Stacktrace:
 [1] optimize!(::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Utilities.UniversalFallback{JuMP.JuMPMOIModel{Float64}}}) at /home/rdeits/locomotion/explorations/jump-lcp/packages/v0.6/MathOptInterface/src/Utilities/cachingoptimizer.jl:155
 [2] #optimize#95(::Bool, ::Function, ::JuMP.Model) at /home/rdeits/locomotion/explorations/jump-lcp/packages/v0.6/JuMP/src/optimizerinterface.jl:48
 [3] optimize(::JuMP.Model) at /home/rdeits/locomotion/explorations/jump-lcp/packages/v0.6/JuMP/src/optimizerinterface.jl:37

If I convert the objective from a single variable to a Linear with:

@objective m Min x + 1

then everything works.

@rdeits rdeits mentioned this pull request May 10, 2018
12 tasks
@testset "LP solver" begin
config = MOIT.TestConfig()
solver = GLPKOptimizerLP()
MOIT.unittest(solver, config, ["solve_singlevariable_obj"])
Copy link
Member Author

Choose a reason for hiding this comment

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

@rdeits yes, single variable objectives aren't supported yet. They need to be implemented in LQOI.

There is a variety of other stuff not implemented yet as well. See
JuliaOpt/LinQuadOptInterface.jl#8

@rdeits
Copy link
Collaborator

rdeits commented May 10, 2018

Got it, thanks!

@coveralls
Copy link

coveralls commented Jun 4, 2018

Coverage Status

Coverage decreased (-78.5%) to 0.0% when pulling b075666 on odow:oscar into 0da1a79 on JuliaOpt:master.

@odow
Copy link
Member Author

odow commented Jun 27, 2018

Hmm @mlubin @joaquimg we have a problem. Because the MOI tests incorrectly allow adding multiple integer constraints (see jump-dev/MathOptInterface.jl#404, JuliaOpt/LinQuadOptInterface.jl#30, JuliaOpt/LinQuadOptInterface.jl#31), this PR now fails tests.

We can either

  • disable the failing tests here
  • fix MOI tests and push a new release
  • merge as is with failing tests.

Thoughts?

@mlubin
Copy link
Member

mlubin commented Jun 27, 2018

Both "disable the failing tests here" (with a TODO explaining why) and "fix MOI tests and push a new release" are viable options.

@odow
Copy link
Member Author

odow commented Jun 28, 2018

Okay this is failing v0.7 but now passing tests on 0.6

@blegat
Copy link
Member

blegat commented Jun 28, 2018

We can target v0.7 for a next release

@odow
Copy link
Member Author

odow commented Jun 28, 2018

This also broke 32-bit because of jump-dev/MathOptInterface.jl#377

@odow odow merged commit 5155ffb into jump-dev:master Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants