Skip to content

Fix slow MOI merging step #129

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

Merged
merged 2 commits into from
Feb 2, 2021
Merged

Fix slow MOI merging step #129

merged 2 commits into from
Feb 2, 2021

Conversation

migarstka
Copy link
Member

  • The current way I merge k COSMO.Nonnegatives(1) into one COSMO.Nonnegatives(k)
    is extremely slow
  • This fix also removes constraint support for the scalar sets as that should
    be handled by MOI.Bridges
  • 1 MOI unit test still errors

- The current way I merge k COSMO.Nonnegatives(1) into one COSMO.Nonnegatives(k)
is extremly slow
- This fix also removes constraint support for the scalar sets as that should
be handled by MOI.Bridges
- 1 MOI unit tests still errors
@migarstka
Copy link
Member Author

@blegat One of the MOI unit tests errors. But I don't understand what's going on. It seems to happen when the constraint in linear14 is deleted. With this error message:

The index MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.Interval{Float64}}(3) is invalid. Note that an index becomes invalid after it has been deleted.linear14: Error During Test at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Test/config.jl:47
  Got exception outside of a @test

  Stacktrace:
   [1] throw_if_not_valid at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/indextypes.jl:75 [inlined]
   [2] delete(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.Interval{Float64}}) at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/bridge_optimizer.jl:410
   [3] delete at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/Constraint/set_map.jl:38 [inlined]
   [4] #16 at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/bridge_optimizer.jl:419 [inlined]
   [5] call_in_context(::MathOptInterface.Bridges.Variable.Map, ::Int64, ::MathOptInterface.Bridges.var"#16#17"{MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}},MathOptInterface.Bridges.Constraint.LessToIntervalBridge{Float64,MathOptInterface.SingleVariable}}) at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/Variable/map.jl:404
   [6] call_in_context(::MathOptInterface.Bridges.Variable.Map, ::MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.LessThan{Float64}}, ::Function) at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/Variable/map.jl:435
   [7] delete(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.LessThan{Float64}}) at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/bridge_optimizer.jl:419
   [8] _delete_variables_in_variables_constraints(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::Array{MathOptInterface.VariableIndex,1}) at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/bridge_optimizer.jl:350
   [9] delete(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::Array{MathOptInterface.VariableIndex,1}) at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/bridge_optimizer.jl:356
   [10] linear14test(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::MathOptInterface.Test.TestConfig{Float64}) at /Users/Micha/.julia/packages/MathOptInterface/ZJFKw/src/Test/contlinear.jl:1653

I guess it's related to the fact that I removed direct support for some constraints:

COSMO.jl/src/MOIWrapper.jl

Lines 788 to 789 in 142fcfd

MOI.supports_constraint(optimizer::Optimizer, ::Type{<:Affine}, ::Type{<: Interval}) = true
MOI.supports_constraint(optimizer::Optimizer, ::Type{<:Union{VectorOfVariables, VectorAffine}}, ::Type{<:SupportedVectorSets}) = true

@odow
Copy link
Contributor

odow commented Feb 1, 2021

@migarstka
Copy link
Member Author

migarstka commented Feb 2, 2021

So COSMO supports VectorAffineFunction + vectorsets and ScalarAffineFunction + Interval.
The problem has a couple of one-sided scalar inequality constraints which get bridged to MOI.Interval. Then when one of the constraints gets deleted, I assume MOI tries to turn the Interval constraint into a SingleVariable-in-LessThan or GreaterThan which might cause the error. Shouldn't MOI try to change the constraint to 1d- VectoAffineFunction-in- Nonnegatives, i.e. something that the optimizer supports? (I know this is a super-edge-case. I still think a multi-dimensional Intervall constraint set in MOI would be nice.)

@migarstka migarstka added the performance Performance issues / improvements label Feb 2, 2021
- Prevent error in linear14 due to the fact that only
ScalarAffine-in-Intervall was supported but not
ScalarAffine-in-LessThan
@blegat
Copy link
Contributor

blegat commented Feb 2, 2021

I know this is a super-edge-case. I still think a multi-dimensional Interval constraint set in MOI would be nice

Would the box constraint support infinite bounds (related to jump-dev/MathOptInterface.jl#1228) ? I'm also in favor of having one Box constraint in MOI.

The problem has a couple of one-sided scalar inequality constraints which get bridged to MOI.Interval

I don't think that's possible, each of them are bridged to a separate Interval constraint.

Shouldn't MOI try to change the constraint to 1d- VectoAffineFunction-in- Nonnegatives, i.e. something that the optimizer supports?

You can check what happens with MOI.Bridges.print_graph, can you print the output ?

@migarstka
Copy link
Member Author

migarstka commented Feb 2, 2021

Infinite bounds wouldn't be a problem for us. The box would effectively be a Nonnegatives/Nonpositives.

This is before MOI.delete is called (without support for GreaterThan/LessThan):

Bridge graph with 3 variable nodes, 8 constraint nodes and 0 objective nodes.
 [1] constrained variables in `MOI.GreaterThan{Float64}` are bridged (distance 1) by MOIB.Variable.VectorizeBridge{Float64,MOI.Nonnegatives}.
 [2] constrained variables in `MOI.Nonpositives` are bridged (distance 1) by MOIB.Variable.NonposToNonnegBridge{Float64}.
 [3] constrained variables in `MOI.LessThan{Float64}` are bridged (distance 2) by MOIB.Variable.VectorizeBridge{Float64,MOI.Nonpositives}.
 (1) `MOI.ScalarAffineFunction{Float64}`-in-`MOI.LessThan{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.LessToIntervalBridge{Float64,MOI.ScalarAffineFunction{Float64}}.
 (2) `MOI.ScalarAffineFunction{Float64}`-in-`MOI.GreaterThan{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.GreaterToIntervalBridge{Float64,MOI.ScalarAffineFunction{Float64}}.
 (3) `MOI.SingleVariable`-in-`MOI.GreaterThan{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.VectorizeBridge{Float64,MOI.VectorAffineFunction{Float64},MOI.Nonnegatives,MOI.SingleVariable}.
 (4) `MOI.SingleVariable`-in-`MOI.Interval{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.ScalarFunctionizeBridge{Float64,MOI.Interval{Float64}}.
 (5) `MOI.SingleVariable`-in-`MOI.LessThan{Float64}` constraints are bridged (distance 2) by MOIB.Constraint.LessToIntervalBridge{Float64,MOI.SingleVariable}.
 (6) `MOI.VectorAffineFunction{Float64}`-in-`MOI.Nonpositives` constraints are bridged (distance 1) by MOIB.Constraint.NonposToNonnegBridge{Float64,MOI.VectorAffineFunction{Float64},MOI.VectorAffineFunction{Float64}}.
 (7) `MOI.VectorOfVariables`-in-`MOI.Nonpositives` constraints are bridged (distance 1) by MOIB.Constraint.NonposToNonnegBridge{Float64,MOI.VectorAffineFunction{Float64},MOI.VectorOfVariables}.
 (8) `MOI.ScalarAffineFunction{Float64}`-in-`MOI.EqualTo{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.VectorizeBridge{Float64,MOI.VectorAffineFunction{Float64},MOI.Zeros,MOI.ScalarAffineFunction{Float64}}.

This is before MOI.delete is called (with GreaterThan/LessThan support)

julia> MOI.Bridges.print_graph(model)
Bridge graph with 1 variable nodes, 5 constraint nodes and 0 objective nodes.
 [1] constrained variables in `MOI.Nonpositives` are bridged (distance 1) by MOIB.Variable.NonposToNonnegBridge{Float64}.
 (1) `MOI.SingleVariable`-in-`MOI.GreaterThan{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.GreaterToLessBridge{Float64,MOI.ScalarAffineFunction{Float64},MOI.SingleVariable}.
 (2) `MOI.SingleVariable`-in-`MOI.Interval{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.ScalarFunctionizeBridge{Float64,MOI.Interval{Float64}}.
 (3) `MOI.SingleVariable`-in-`MOI.LessThan{Float64}` constraints are bridged (distance 1) by MOIB.Constraint.LessToGreaterBridge{Float64,MOI.ScalarAffineFunction{Float64},MOI.SingleVariable}.
 (4) `MOI.VectorAffineFunction{Float64}`-in-`MOI.Nonpositives` constraints are bridged (distance 1) by MOIB.Constraint.NonposToNonnegBridge{Float64,MOI.VectorAffineFunction{Float64},MOI.VectorAffineFunction{Float64}}.
 (5) `MOI.VectorOfVariables`-in-`MOI.Nonpositives` constraints are bridged (distance 1) by MOIB.Constraint.NonposToNonnegBridge{Float64,MOI.VectorAffineFunction{Float64},MOI.VectorOfVariables}.

@blegat
Copy link
Contributor

blegat commented Feb 2, 2021

I can reduce it to

model = MOI.instantiate(COSMO.Optimizer, with_bridge_type=Float64)
MOI.empty!(model)
x = MOI.add_variable(model)
c = MOI.add_constraint(model, MOI.SingleVariable(x), MOI.LessThan(1.0))
MOI.delete(model, x)

and with debug

model = MOI.instantiate(COSMO.Optimizer, with_bridge_type=Float64)
MOI.empty!(model)
x = MOI.add_variable(model)
c = MOI.add_constraint(model, MOI.SingleVariable(x), MOI.LessThan(1.0))
b = MOI.Bridges.bridge(model, c)
@show typeof(b)
@show b.constraint
b2 = MOI.Bridges.bridge(model, b.constraint)
@show typeof(b2)
@show b2.constraint
@show MOI.Bridges.is_bridged(model, b2.constraint)
MOI.delete(model, x)

which gives

typeof(b) = MathOptInterface.Bridges.Constraint.LessToIntervalBridge{Float64,MathOptInterface.SingleVariable}
b.constraint = MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.Interval{Float64}}(1)
typeof(b2) = MathOptInterface.Bridges.Constraint.ScalarFunctionizeBridge{Float64,MathOptInterface.Interval{Float64}}
b2.constraint = MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.Interval{Float64}}(1)
MOI.Bridges.is_bridged(model, b2.constraint) = false
ERROR: LoadError: The index MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.Interval{Float64}}(1) is invalid. Note that an index becomes invalid after it has been deleted.
Stacktrace:
 [1] throw_if_not_valid at /home/blegat/.julia/dev/MathOptInterface/src/indextypes.jl:75 [inlined]
 [2] delete(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.Interval{Float64}}) at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/bridge_optimizer.jl:461
 [3] delete at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/Constraint/set_map.jl:53 [inlined]
 [4] #16 at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/bridge_optimizer.jl:475 [inlined]
 [5] call_in_context(::MathOptInterface.Bridges.Variable.Map, ::Int64, ::MathOptInterface.Bridges.var"#16#17"{MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}},MathOptInterface.Bridges.Constraint.LessToIntervalBridge{Float64,MathOptInterface.SingleVariable}}) at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/Variable/map.jl:443
 [6] call_in_context(::MathOptInterface.Bridges.Variable.Map, ::MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.LessThan{Float64}}, ::Function) at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/Variable/map.jl:474
 [7] delete(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::MathOptInterface.ConstraintIndex{MathOptInterface.SingleVariable,MathOptInterface.LessThan{Float64}}) at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/bridge_optimizer.jl:472
 [8] _delete_variables_in_variables_constraints(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::Array{MathOptInterface.VariableIndex,1}) at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/bridge_optimizer.jl:395
 [9] delete(::MathOptInterface.Bridges.LazyBridgeOptimizer{MathOptInterface.Utilities.CachingOptimizer{Optimizer{Float64},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}}, ::MathOptInterface.VariableIndex) at /home/blegat/.julia/dev/MathOptInterface/src/Bridges/bridge_optimizer.jl:429
 [10] top-level scope at /home/blegat/.julia/dev/COSMO/tmp.jl:14
 [11] include(::String) at ./client.jl:457
 [12] top-level scope at REPL[3]:1
in expression starting at /home/blegat/.julia/dev/COSMO/tmp.jl:14

@blegat
Copy link
Contributor

blegat commented Feb 2, 2021

This is an MOI bug, I've opened an issue there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance issues / improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants