-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
Bridge backend of Model if needed #2610
Conversation
|
Codecov Report
@@ Coverage Diff @@
## master #2610 +/- ##
==========================================
- Coverage 94.18% 93.96% -0.23%
==========================================
Files 43 43
Lines 5522 5564 +42
==========================================
+ Hits 5201 5228 +27
- Misses 321 336 +15
Continue to review full report at Codecov.
|
Docs are failing because of jump-dev/MathOptInterface.jl#1329. This shows that we're copying straight to GLPK, which should give a nice speedup though! Alternate fix here: jump-dev/GLPK.jl#183 |
cc43c04
to
d2f253c
Compare
Good news on the GLPK front, we're < 4 seconds, compared to 15 seconds in May.
|
c5283eb
to
b4c8f83
Compare
2569bf7
to
9c04228
Compare
|
22f844d
to
ad23255
Compare
@@ -186,6 +205,16 @@ function optimize!( | |||
"The solver does not support nonlinear problems " * | |||
"(i.e., NLobjective and NLconstraint).", | |||
) | |||
elseif err isa MOI.UnsupportedConstraint | |||
if _add_bridges_if_needed(model) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we also catch unsupported objective function ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also free variables that are not supported ? You should probably try with CSDP see the error that is thrown
end | ||
|
||
model = direct_model( | ||
MOI.Utilities.CachingOptimizer( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we could directly do direct_model(MOIU.UniversalFallback(MOIU.Model{Float64}()))
. Otherwise we will compile a lot of methods for MOIU.CachingOptimizer{...,MOI.AbstractOptimizer}
but we will never use the fact that it's a caching optimizer. If we use direct_model(MOIU.UniversalFallback(MOIU.Model{Float64}()))
directly, wouldn't we gain in compile time ?
It won't add more possible states. Currently the possible states are:
- The MOI backend is a CachingOptimizer in state NO_OPTIMIZER
- An optimizer is set
and if we do the change I suggest, the possible states are - The MOI backend is
UniversalFallback{Model}
- An optimizer is set and the MOI backend is a CachingOptimizer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work for two reasons:
UniversalFallback
is not anAbstractOptimizer
- We wouldn't be able to tell if the user was in direct mode
Rebase and fix formatting bridge_formulation instead of bridge_constraints Some test fixes More fixes Fix docs. Requires new GLPK release Update models.md Remove TODO Another fix More fixes Fix formatting Fix typo More tests Update solvers_and_solutions.jl Rename bridge_formulation to force_bridge_formulation
I'm having second thoughts about this PR. Perhaps we just need to:
|
This PR addresses the "time-to-first-solve" issue (jump-dev/MathOptInterface.jl#1313). It is essentially a repeat of jump-dev/MathOptInterface.jl#1252, but at the JuMP level instead of MOI.
Closes
Closes #2520
Closes #1627
Background
The main issues are:
Demo
By default, the
LazyBridgeOptimizer
is omitted. But, if you do something that requires bridges, they are added:You can also pass
force_bridge_formulation = true
to force-add the bridges:Benchmark
Current
master
This PR
The big change is really that its 2x as fast when bridges are not needed.
Alternative
The alternative is to change the default of
bridge_constraints
tofalse
, and also fix the double abstract-type issue.This would give everyone a speed-up if they don't use bridges, and for models which use bridges, they can just go
This would have the added benefit of giving people greater insight into when bridges get used.
TODO
TODOs before merging