Duplicate constraint name errors with JuMP

[exaexa]

Hello, recently, we saw Tulip errors in https://github.com/LCSB-BioCore/COBREXA.jl, likely caused by update to JuMP 0.22.x. Since no error is triggered with other optimizers, I assume this might be a potential problem in Tulip. Basically, optimizing any LP with named constraints causes an error Duplicate constraint name. The demonstration with only JuMP (0.22.1)+Tulip (0.9.1) is below.

I hope this is a fixable issue (looks like some kind of a logic mistake in name checking to me). Please let me know if I can supply any debugging information.

Thanks for any help! -mk

julia> using Tulip, JuMP

julia> m = Model(Tulip.Optimizer)
A JuMP Model
Feasibility problem with:
Variables: 0
Model mode: AUTOMATIC
CachingOptimizer state: EMPTY_OPTIMIZER
Solver name: Tulip

julia> @variable(m, x[i=1:2])
2-element Vector{VariableRef}:
 x[1]
 x[2]

julia> @constraint(m, mb, x[1]+x[2]==1)
mb : x[1] + x[2] = 1.0

julia> @constraint(m, lbs, [0,0] .<= x)
2-element Vector{ConstraintRef{Model, MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.LessThan{Float64}}, ScalarShape}}:
 lbs : -x[1] ≤ 0.0
 lbs : -x[2] ≤ 0.0

julia> @constraint(m, ubs, [1,1] .>= x)
2-element Vector{ConstraintRef{Model, MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}, ScalarShape}}:
 ubs : -x[1] ≥ -1.0
 ubs : -x[2] ≥ -1.0

julia> @objective(m, Max, [1,0.5]' * x)
x[1] + 0.5 x[2]

julia> optimize!(m)
ERROR: Duplicate constraint name ubs
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:33
  [2] set(m::Tulip.Optimizer{Float64}, #unused#::MathOptInterface.ConstraintName, c::MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}, name::String)
    @ Tulip ~/.julia/packages/Tulip/VjKXM/src/Interfaces/MOI/constraints.jl:395
  [3] set
    @ ~/.julia/packages/MathOptInterface/QxT5e/src/Bridges/bridge_optimizer.jl:1362 [inlined]
  [4] _pass_attribute(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, index_map::MathOptInterface.Utilities.IndexMap, cis_src::Vector{MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}}, attr::MathOptInterface.ConstraintName)
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:137
  [5] pass_attributes(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, index_map::MathOptInterface.Utilities.IndexMap, cis_src::Vector{MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:122
  [6] _pass_constraints(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, index_map::MathOptInterface.Utilities.IndexMap, variable_constraints_not_added::Vector{Any})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:316
  [7] default_copy_to(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:498
  [8] #copy_to#7
    @ ~/.julia/packages/MathOptInterface/QxT5e/src/Bridges/bridge_optimizer.jl:421 [inlined]
  [9] copy_to
    @ ~/.julia/packages/MathOptInterface/QxT5e/src/Bridges/bridge_optimizer.jl:421 [inlined]
 [10] optimize!(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}})
    @ MathOptInterface ~/.julia/packages/MathOptInterface/QxT5e/src/MathOptInterface.jl:80
 [11] optimize!(m::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/cachingoptimizer.jl:285
 [12] optimize!(model::Model, optimizer_factory::Nothing; bridge_constraints::Bool, ignore_optimize_hook::Bool, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ JuMP ~/.julia/packages/JuMP/lnUbA/src/optimizer_interface.jl:195
 [13] optimize! (repeats 2 times)
    @ ~/.julia/packages/JuMP/lnUbA/src/optimizer_interface.jl:167 [inlined]
 [14] top-level scope
    @ REPL[13]:1

[mtanneau]

Hi! Thanks for reporting this. I have never encountered that issue 🤔

At first sight, it looks like the following happens:

1. The @constraint calls in JuMP end up creating two separate constraints with the same name.
2. The constraints are copied from the JuMP cache to the Tulip internal data structures...
3. ... which throws an error when two constraints with the same name are added

Let me look into it.

[exaexa]

Thanks. Just to add a few version observations:

• we were using Tulip 0.7 and JuMP 0.21.10 before, that is okay
• Tulip 0.8 vs JuMP 0.21.10 works
• Tulip 0.9.1 vs JuMP 0.22.{1,2} fails

[exaexa]

Some debugging info: I added:

    @info "in MOI.set constraint name" T c c_ name string(m.name2con)
    c_ === nothing || c_ == c || error("Duplicate constraint name $name")

testing:

using Tulip, JuMP
m = Model(Tulip.Optimizer)
@variable(m, x[i=1:2])
@info "set con 1"
@constraint(m, mb, x[1]+x[2]==1)
@info "set con 2"
@constraint(m, lbs, [0,0] .<= x)
@info "set con 3"
@constraint(m, ubs, [1,1] .>= x)
@info "set obj"
@objective(m, Max, [1,0.5]' * x)
@info "optim"
optimize!(m)
@info "res" value.(m[:x])

gives log:

[ Info: set con 1
[ Info: set con 2
[ Info: set con 3
[ Info: set obj
[ Info: optim
┌ Info: in MOI.set constraint name
│   T = Float64
│   c = MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.EqualTo{Float64}}(1)
│   c_ = nothing
│   name = "mb"
└   string(m.name2con) = "Dict{String, MathOptInterface.ConstraintIndex}()"
┌ Info: in MOI.set constraint name
│   T = Float64
│   c = MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}(2)
│   c_ = nothing
│   name = "ubs"
└   string(m.name2con) = "Dict{String, MathOptInterface.ConstraintIndex}(\"mb\" => MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.EqualTo{Float64}}(1))"
┌ Info: in MOI.set constraint name
│   T = Float64
│   c = MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}(3)
│   c_ = MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}(2)
│   name = "ubs"
└   string(m.name2con) = "Dict{String, MathOptInterface.ConstraintIndex}(\"ubs\" => MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}(2), \"mb\" => MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.EqualTo{Float64}}(1))"
ERROR: LoadError: Duplicate constraint name ubs
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:33
  [2] set(m::Tulip.Optimizer{Float64}, #unused#::MathOptInterface.ConstraintName, c::MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}, name::String)
    @ Tulip ~/work/Tulip.jl/src/Interfaces/MOI/constraints.jl:397
  [3] set
    @ ~/.julia/packages/MathOptInterface/QxT5e/src/Bridges/bridge_optimizer.jl:1362 [inlined]
  [4] _pass_attribute(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, index_map::MathOptInterface.Utilities.IndexMap, cis_src::Vector{MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}}, attr::MathOptInterface.ConstraintName)
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:137
  [5] pass_attributes(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, index_map::MathOptInterface.Utilities.IndexMap, cis_src::Vector{MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.GreaterThan{Float64}}})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:122
  [6] _pass_constraints(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}, index_map::MathOptInterface.Utilities.IndexMap, variable_constraints_not_added::Vector{Any})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:316
  [7] default_copy_to(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/copy.jl:498
  [8] #copy_to#7
    @ ~/.julia/packages/MathOptInterface/QxT5e/src/Bridges/bridge_optimizer.jl:421 [inlined]
  [9] copy_to
    @ ~/.julia/packages/MathOptInterface/QxT5e/src/Bridges/bridge_optimizer.jl:421 [inlined]
 [10] optimize!(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, src::MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}})
    @ MathOptInterface ~/.julia/packages/MathOptInterface/QxT5e/src/MathOptInterface.jl:80
 [11] optimize!(m::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.Bridges.LazyBridgeOptimizer{Tulip.Optimizer{Float64}}, MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/QxT5e/src/Utilities/cachingoptimizer.jl:285
 [12] optimize!(model::Model, optimizer_factory::Nothing; bridge_constraints::Bool, ignore_optimize_hook::Bool, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ JuMP ~/.julia/packages/JuMP/lnUbA/src/optimizer_interface.jl:195
 [13] optimize! (repeats 2 times)
    @ ~/.julia/packages/JuMP/lnUbA/src/optimizer_interface.jl:167 [inlined]
 [14] top-level scope
    @ ~/work/COBREXA.jl/test.jl:13
in expression starting at /home/exa/work/COBREXA.jl/test.jl:13

[exaexa]

I've been trying to debug this a bit and it looks like either MOI or JuMP are for whatever reason calling the Tulip backend with erroneous constraint name for the first time (the first one added is lbs). I hope this isn't going to be some kind of weird referencing error.

EDIT: I can't deny that the constraint duplication as you described doesn't happen but I haven't been able to track it down yet.

[mtanneau]

Sorry, this has been on the back burner. Thanks for all the useful debugging info!

Diagnostic

I compared Tulip's internals to the wrapper in Gurobi.jl. The main difference is the following:

• In Tulip, when you create two constraints with the same name, it throws an error
• In Gurobi.jl, you can create two constraints with the same name, but you will get an error if you try to query either by name.

For instance,

m = direct_model(Tulip.Optimizer())
moi = backend(m)
@variable(m, x[1:2])
@constraint(m, lbs, [0, 0] .<= x)  # This line will error
@info "Constraint 'lbs' created"
MOI.get(moi, MOI.ConstraintIndex, "lbs")

# Error message
# ERROR: Duplicate constraint name lbs

m = direct_model(Gurobi.Optimizer())
moi = backend(m)
@variable(m, x[1:2])
@constraint(m, lbs, [0, 0] .<= x)  # this line is fine...
MOI.get(moi, MOI.ConstraintIndex, "lbs")  # ...but this line errors

# Error message
# ERROR: Duplicate constraint name detected: lbs

The root difference is that, in Tulip, I maintain a name => constraint mapping (Dict), and I throw an error as soon as I detect you're trying to create duplicate constraint names. In Gurobi.jl, a name => constraint mapping (Dict) is also kept, but it will let you create a constraint with duplicate name. When that happens, the constraint value in that mapping is changed to nothing (to mark that there's a duplicate). Then, when you try to query a constraint by name, it throws an error if it detects a duplicate.

In other words, in Tulip, duplicate names are not allowed, whereas in Gurobi.jl, they're allowed as long as you don't try to do an ambiguous query.

I don't know how the recent JuMP/MOI release affected their behavior w.r.t names.

Solution

TBH, I don't like having duplicate constraint names in the first place. That would potentially cause issues when manually inspecting/writing the model later. However, I do want people to be able to use Tulip via JuMP, and I'm not going to change how MOI/JuMP handle constraint names.

I'll update the internals to do something similar to Gurobi.jl.

cc-ing @odow in case a similar issue pops up somewhere else. (nothing to do, just for for reference)

[exaexa]

OK, good to know. It certainly sounds weird that a single named "vector" of constraints should make multiple small ones, e.g. we've been using this code happily with JuMP to modify some of the constraints selectively: https://github.com/LCSB-BioCore/COBREXA.jl/blob/master/src/base/solver.jl#L82-L83

If I get it right, this kind of referencing won't be possible with new JuMP anymore?

[mtanneau]

OK, good to know. It certainly sounds weird that a single named "vector" of constraints should make multiple small ones, e.g. we've been using this code happily with JuMP to modify some of the constraints selectively: https://github.com/LCSB-BioCore/COBREXA.jl/blob/master/src/base/solver.jl#L82-L83

If I get it right, this kind of referencing won't be possible with new JuMP anymore?

That should still work, because something like model[:lbs] does not query constraints by their MOI.ConstraintName attribute. Instead, it's like a dictionary query, and it will return the object attached to the JuMP model whose name is :lbs. This may be anything: a variable container, an expression, a constraint container, or something you've manually registered. It doesn't hit the MOI backend.

[exaexa]

That should still work

OK that would be great :D Is there some straightforward workaround to make it work like this that doesn't require you to break the unique naming logic in Tulip?

[odow]

Okay, I'll try to address most of the points since a lot of this is my fault.

The PR that changed JuMP was: https://github.com/jump-dev/JuMP.jl/pull/2739

We now set the same name for each row in a vectorized constraint.

MOI requires that:

• Duplicate names are accepted on set
• Duplicate names thrown an error on get

Docs: https://jump.dev/MathOptInterface.jl/stable/tutorials/implementing/#implement_names

In other words, in Tulip, duplicate names are not allowed, whereas in Gurobi.jl, they're allowed as long as you don't try to do an ambiguous query.

Gurobi.jl behavior is correct, and was purposefully chosen to be this way.

I don't like having duplicate constraint names in the first place. That would potentially cause issues when manually inspecting/writing the model later.

The LP and MPS writers have code to make all names unique. So two variables called x will get renamed to x and x_1.

If I get it right, this kind of referencing won't be possible with new JuMP anymore?

It is still possible. The only thing that changed is you can't access constraint_by_name(model, "lhs"), but you couldn't do that previously because the names weren't passed to the solver.

[exaexa]

We just checked, everything seems to work perfectly with 0.9.2. Many thanks for fixing so quickly!