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

VectorOfVariable to VectorAffineFunction does not keep variable indices? #2167

Closed
matbesancon opened this issue Feb 17, 2020 · 10 comments
Closed

Comments

@matbesancon
Copy link
Contributor

MWE:

using JuMP
import SCIP
using LinearAlgebra: 

m = Model(with_optimizer(SCIP.Optimizer))
nl = 10

@variable(m, v[1:nl])
@variable(m, s[j=1:nl])
@variable(m, z[j=1:nl], Bin)

@assert all(JuMP.is_binary, z)

@constraint(m,
    indicator_ones[j=1:nl],
    [z[j], v[j]] in MOI.IndicatorSet{MOI.ACTIVATE_ON_ONE}(MOI.LessThan(0.0))
)
optimize!(m)

The problem is fixed by:

@constraint(m,
    indicator_ones[j=1:nl],
    [z[j], 1.0 * v[j]] in MOI.IndicatorSet{MOI.ACTIVATE_ON_ONE}(MOI.LessThan(0.0))
)

The error out at the SCIP level, with the following trace:

julia> optimize!(m)
ERROR: y variable must be binary for indicator constraint
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] add_indicator_constraint(::SCIP.ManagedSCIP, ::SCIP.VarRef, ::Array{SCIP.VarRef,1}, ::Array{Float64,1}, ::Float64) at /home/mbesancon/.julia/packages/SCIP/hrirX/src/managed_scip.jl:319
 [3] add_constraint(::SCIP.Optimizer, ::MathOptInterface.VectorAffineFunction{Float64}, ::MathOptInterface.IndicatorSet{MathOptInterface.ACTIVATE_ON_ONE,MathOptInterface.LessThan{Float64}}) at /home/mbesancon/.julia/packages/SCIP/hrirX/src/MOI_wrapper/indicator_constraints.jl:14
 [4] add_constraint at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Bridges/bridge_optimizer.jl:1057 [inlined]
 [5] bridge_constraint(::Type{MathOptInterface.Bridges.Constraint.VectorFunctionizeBridge{Float64,MathOptInterface.IndicatorSet{MathOptInterface.ACTIVATE_ON_ONE,MathOptInterface.LessThan{Float64}}}}, ::MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer}, ::MathOptInterface.VectorOfVariables, ::MathOptInterface.IndicatorSet{MathOptInterface.ACTIVATE_ON_ONE,MathOptInterface.LessThan{Float64}}) at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Bridges/Constraint/functionize.jl:66
 [6] add_bridged_constraint(::MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer}, ::Type, ::MathOptInterface.VectorOfVariables, ::MathOptInterface.IndicatorSet{MathOptInterface.ACTIVATE_ON_ONE,MathOptInterface.LessThan{Float64}}) at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Bridges/bridge_optimizer.jl:1006
 [7] add_constraint(::MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer}, ::MathOptInterface.VectorOfVariables, ::MathOptInterface.IndicatorSet{MathOptInterface.ACTIVATE_ON_ONE,MathOptInterface.LessThan{Float64}}) at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Bridges/bridge_optimizer.jl:1055
 [8] copy_vector_of_variables(::MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer}, ::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, ::MathOptInterface.Utilities.IndexMap, ::Type{MathOptInterface.IndicatorSet{MathOptInterface.ACTIVATE_ON_ONE,MathOptInterface.LessThan{Float64}}}) at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Bridges/bridge_optimizer.jl:1165
 [9] (::MathOptInterface.Utilities.var"#126#134"{MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}},MathOptInterface.Utilities.IndexMap})(::Type) at ./none:0
 [10] collect(::Base.Generator{Array{DataType,1},MathOptInterface.Utilities.var"#126#134"{MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer},MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}},MathOptInterface.Utilities.IndexMap}}) at ./generator.jl:47
 [11] default_copy_to(::MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer}, ::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, ::Bool) at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Utilities/copy.jl:321
 [12] #automatic_copy_to#109 at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Utilities/copy.jl:15 [inlined]
 [13] #automatic_copy_to at ./none:0 [inlined]
 [14] #copy_to#3 at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Bridges/bridge_optimizer.jl:268 [inlined]
 [15] (::MathOptInterface.var"#kw##copy_to")(::NamedTuple{(:copy_names,),Tuple{Bool}}, ::typeof(MathOptInterface.copy_to), ::MathOptInterface.Bridges.LazyBridgeOptimizer{SCIP.Optimizer}, ::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}) at ./none:0
 [16] attach_optimizer(::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.AbstractOptimizer,MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}) at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Utilities/cachingoptimizer.jl:149
 [17] optimize!(::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.AbstractOptimizer,MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}) at /home/mbesancon/.julia/packages/MathOptInterface/DmQBj/src/Utilities/cachingoptimizer.jl:185
 [18] #optimize!#97(::Bool, ::Bool, ::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::typeof(optimize!), ::Model, ::Nothing) at /home/mbesancon/.julia/packages/JuMP/Sp4sR/src/optimizer_interface.jl:131
 [19] optimize! at /home/mbesancon/.julia/packages/JuMP/Sp4sR/src/optimizer_interface.jl:107 [inlined] (repeats 2 times)
 [20] top-level scope at REPL[168]:1
@matbesancon
Copy link
Contributor Author

Versions are as follow:

  [4076af6c] JuMP v0.21.0
  [b8f27783] MathOptInterface v0.9.10
  [82193955] SCIP v0.9.1

@odow
Copy link
Member

odow commented Feb 18, 2020

Hmm. Is this because the copy_to is adding the vector-of-variables in indicator constraint before it adds the single-variable in binary constraint?

We should probably copy single-variable constraints first:
https://github.com/JuliaOpt/MathOptInterface.jl/blob/fb303dceaa85dfff8f97e7fe13542c70fdef7754/src/Utilities/copy.jl#L321-L328

But argh. That's not very robust. @blegat, this ties into a lot of things we've been talking about, relating to the order in which constraints are added.

@matbesancon
Copy link
Contributor Author

Should we transfer this issue to MOI then? I couldn't reproduce the bug without JuMP, that's why I initially put it here

@blegat
Copy link
Member

blegat commented Feb 18, 2020

We can add it as a motivating example for jump-dev/MathOptInterface.jl#987.
If SCIP requires binary variables to be added as constrained variables then once jump-dev/MathOptInterface.jl#987 is fixed, binary variables will always be added first.
Even if SCIP does not require then to be added as constrained variables, it might work in most cases as the VoV-in-Indicator are bridges into VAF-in-Indicator so this won't have priority to be added as constrained variable because of the bridge cost.

@odow
Copy link
Member

odow commented Feb 18, 2020

I guess the other option is to make SCIP set y to binary, regardless of an existing SingleVariable-in-ZeroOne constraint.

@blegat
Copy link
Member

blegat commented Feb 19, 2020

Yes, and error at optimize! if it was not explicitly set to binary afterwards.

@odow
Copy link
Member

odow commented Feb 18, 2021

@matbesancon can this issue be closed and re-opened in SCIP.jl then?

@odow
Copy link
Member

odow commented Mar 31, 2021

Bump @matbesancon

@matbesancon
Copy link
Contributor Author

yes I will add the issue in SCIP.jl. I see the MOI issues posted by @blegat are closed

@odow
Copy link
Member

odow commented Mar 31, 2021

Closing in favor of scipopt/SCIP.jl#191

@odow odow closed this as completed Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants