Fix bug broadcasting constraints with sparse terms #2558
Conversation
src/macros.jl
Outdated
| When broadcasting `f.(x)` over an `AbstractSparseArray` `x`, Julia first calls | ||
| the equivalent of `f(zero(eltype(x))`. However, if `f` is mutating, this can | ||
| have serious consequences! In our case, broadcasting `build_constraint` will add | ||
| a new `0 = 0` constraint. |
There was a problem hiding this comment.
How does collect help ? It also replaces the structural zero elements with zeros
There was a problem hiding this comment.
In most cases (all cases?), promotion means that the incoming SparseArray is actually dense.
In 0.21.7, you get
julia> model = Model()
A JuMP Model
Feasibility problem with:
Variables: 0
Model mode: AUTOMATIC
CachingOptimizer state: NO_OPTIMIZER
Solver name: No optimizer attached.
julia> @variable(model, x[1:2])
2-element Vector{VariableRef}:
x[1]
x[2]
julia> d = sparsevec([1, 3], x)
3-element SparseVector{VariableRef, Int64} with 2 stored entries:
[1] = x[1]
[3] = x[2]
julia> @constraint(model, d .== 0)
3-element SparseVector{Any, Int64} with 3 stored entries:
[1] = x[1] = 0.0
[2] = 0 = 0.0
[3] = x[2] = 0.0
julia> model
A JuMP Model
Feasibility problem with:
Variables: 2
`AffExpr`-in-`MathOptInterface.EqualTo{Float64}`: 4 constraints
Model mode: AUTOMATIC
CachingOptimizer state: NO_OPTIMIZER
Solver name: No optimizer attached.
Names registered in the model: xIn this PR, you get
julia> model = Model()
A JuMP Model
Feasibility problem with:
Variables: 0
Model mode: AUTOMATIC
CachingOptimizer state: NO_OPTIMIZER
Solver name: No optimizer attached.
julia> @variable(model, x[1:2])
2-element Vector{VariableRef}:
x[1]
x[2]
julia> d = sparsevec([1, 3], x)
3-element SparseVector{VariableRef, Int64} with 2 stored entries:
[1] = x[1]
[3] = x[2]
julia> @constraint(model, d .== 0)
3-element Vector{ConstraintRef{Model, MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64}, MathOptInterface.EqualTo{Float64}}, ScalarShape}}:
x[1] = 0.0
0 = 0.0
x[2] = 0.0
julia> model
A JuMP Model
Feasibility problem with:
Variables: 2
`AffExpr`-in-`MathOptInterface.EqualTo{Float64}`: 3 constraints
Model mode: AUTOMATIC
CachingOptimizer state: NO_OPTIMIZER
Solver name: No optimizer attached.
Names registered in the model: xIs changing the return type of @constraint here breaking? The type change happens with dense constraints too.
Codecov Report
@@ Coverage Diff @@
## master #2558 +/- ##
=======================================
Coverage 93.44% 93.45%
=======================================
Files 43 43
Lines 5404 5406 +2
=======================================
+ Hits 5050 5052 +2
Misses 354 354
Continue to review full report at Codecov.
|
|
What was the conclusion on changing the return type of |
|
I wouldn't consider it breaking. Having a sparse vector of constraint refs can be considered a bug for 2 reasons: 1) zero is not defined for constraint refs. 2) this is dense so it does not make sense to returns it sparse |
odow
force-pushed
the
od/fix-sparse-broadcast
branch
from
f536a6e
to
02a3ac4
Compare
Closes #2162
Needs jump-dev/MutableArithmetics.jl#95