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

Allow MOI.ModelLike as the optimizer instead of MOI.AbstractOptimizer #3667

Merged
merged 4 commits into from
Feb 7, 2024

Conversation

odow
Copy link
Member

@odow odow commented Feb 5, 2024

Closes jump-dev/MathOptInterface.jl#2417

The only downside is this:

julia> model = Model(MOI.FileFormats.MPS.Model)
A JuMP Model
Feasibility problem with:
Variables: 0
Model mode: AUTOMATIC
CachingOptimizer state: EMPTY_OPTIMIZER
Solver name: unknown

julia> @variable(model, x >= 0)
x

julia> optimize!(model)
ERROR: MethodError: no method matching optimize!(::MathOptInterface.Utilities.GenericModel{…})

Closest candidates are:
  optimize!(::Any, ::Any)
   @ MathOptInterface ~/.julia/packages/MathOptInterface/3JqTJ/src/MathOptInterface.jl:82
  optimize!(::MathOptInterface.Bridges.AbstractBridgeOptimizer)
   @ MathOptInterface ~/.julia/packages/MathOptInterface/3JqTJ/src/Bridges/bridge_optimizer.jl:378
  optimize!(::MathOptInterface.Utilities.CachingOptimizer)
   @ MathOptInterface ~/.julia/packages/MathOptInterface/3JqTJ/src/Utilities/cachingoptimizer.jl:302
  ...

Stacktrace:
 [1] optimize!(b::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.GenericModel{…}})

It violates the MethodError principle because we're throwing an error deep in the stack.

Should we check whether the backend is an optimizer?

julia> model = Model(MOI.FileFormats.MPS.Model)
A JuMP Model
Feasibility problem with:
Variables: 0
Model mode: AUTOMATIC
CachingOptimizer state: EMPTY_OPTIMIZER
Solver name: unknown

julia> unsafe_backend(model)
A Mathematical Programming System (MPS) model

julia> unsafe_backend(model) isa MOI.AbstractOptimizer
false

@odow
Copy link
Member Author

odow commented Feb 5, 2024

Now

julia> optimize!(model)
ERROR: Cannot call `optimize!` because the provided optimizer is not a subtype of `MOI.AbstractOptimizer`.
Stacktrace:

I didn't add the type because I don't know if this adds anything:

julia> print(typeof(unsafe_backend(model)))
MathOptInterface.Utilities.GenericModel{Float64, MathOptInterface.Utilities.ObjectiveContainer{Float64}, MathOptInterface.Utilities.VariablesContainer{Float64}, MathOptInterface.FileFormats.MPS.ModelFunctionConstraints{Float64}}

I guess I could show it instead

julia> sprint(show, unsafe_backend(model))
"A Mathematical Programming System (MPS) model"

@odow
Copy link
Member Author

odow commented Feb 5, 2024

julia> optimize!(model)
ERROR: Cannot call `optimize!` because the provided optimizer is not a subtype of `MOI.AbstractOptimizer`.

The optimizer is:

A Mathematical Programming System (MPS) model

Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] optimize!(model::Model; ignore_optimize_hook::Bool, _differentiation_backend::MathOptInterface.Nonlinear.SparseReverseMode, kwargs::@Kwargs{})
   @ JuMP ~/.julia/dev/JuMP/src/optimizer_interface.jl:449
 [3] optimize!(model::Model)
   @ JuMP ~/.julia/dev/JuMP/src/optimizer_interface.jl:409
 [4] top-level scope
   @ REPL[55]:1

src/optimizer_interface.jl Outdated Show resolved Hide resolved
test/test_model.jl Outdated Show resolved Hide resolved
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e4d2d7f) 98.29% compared to head (661dc0a) 98.32%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3667      +/-   ##
==========================================
+ Coverage   98.29%   98.32%   +0.03%     
==========================================
  Files          43       43              
  Lines        5676     5682       +6     
==========================================
+ Hits         5579     5587       +8     
+ Misses         97       95       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

test/test_model.jl Show resolved Hide resolved
Copy link
Member

@mlubin mlubin left a comment

Choose a reason for hiding this comment

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

Looks pretty reasonable. Are there any other consequences of this change?

test/test_model.jl Show resolved Hide resolved
@odow
Copy link
Member Author

odow commented Feb 6, 2024

Are there any other consequences of this change?

I don't think so. All of the solvers are AbstractOptimizer, so passing something that creates a ModelLike is non-trivial, and I expect that people who do so know what and why they're doing it.

But here's a extension-test to confirm: https://github.com/jump-dev/JuMP.jl/actions/runs/7804969693

@odow odow merged commit e2149bf into master Feb 7, 2024
24 of 25 checks passed
@odow odow deleted the od/modellike branch February 7, 2024 20:24
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.

Make FileFormats.MPS.Model an AbstractOptimizer
3 participants