Jump.jl: Should we define zero(AffExpr) and one(AffExpr)?

[mlubin]

The implementation of zeros(AffExpr, n) looks like fill!(Array{AffExpr}(n), zero(AffExpr)), which is problematic because the result is n entries that refer to the same AffExpr (https://github.com/JuliaOpt/JuMP.jl/issues/1113). Same for ones.

This leads me to question if we should define zero(AffExpr) or one(AffExpr) at all given that code may be written assuming that these return immutable objects. The replacement is AffExpr(0.0) and AffExpr(1.0).

@daschw

[blegat]

If Base is doing this, they may be implicitly assuming that only immutable objects should define zero and one.

Maybe we could have two version of AffExpr ? One immutable and one mutable.
The mutability of AffExpr is only used inside JuMP macros so we could take care of using the mutable version there.
If the user do operation on variables outside JuMP macros it will become the immutable version.

[mlubin]

The mutability of AffExpr is only used inside JuMP macros so we could take care of using the mutable version there.

Not true, we also expose push! and some other functions to append to AffExprs. These are pretty useful in performance-critical situations.

[joaquimg]

The replacement is AffExpr(0.0) and AffExpr(1.0)

seems reasonable.

[blegat]

Not true, we also expose push! and some other functions to append to AffExprs. These are pretty useful in performance-critical situations.

We could have a warning push! not supported in AffExpr, please convert it to a MutableAffExpr.

[mlubin]

Having both a mutable and immutable AffExpr is worth considering, but I'd put it off until after 0.19.

[joaquimg]

What is the advantage of having the immutable version?

[mlubin]

This example by @jdlara-berkeley suggests a similar issue with convert:

m = Model()
x = @variable(m, x >= 0)
NetInjectionVar =  Array{AffExpr,2}(undef, 5, 24)
NetInjectionVar[:] .= 0.0
JuMP.add_to_expression!(NetInjectionVar[1,1],x)
NetInjectionVar
5×24 Array{JuMP.GenericAffExpr{Float64,JuMP.VariableRef},2}:
 x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x
 x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x
 x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x
 x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x
 x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x  x

I claim that this would be less of a gotcha if you had to write

NetInjectionVar[:] .= AffExpr(0.0)

instead of

NetInjectionVar[:] .= 0.0

[odow]

Now that we've changed to Dicts, is this still relevant?

m = Model()
x = @variable(m, x >= 0)
NetInjectionVar =  Array{JuMP.AffExpr,2}(undef, 5, 24)
NetInjectionVar[:] .= zero(AffExpr)
NetInjectionVar[1,1] += x

julia> NetInjectionVar
5×24 Array{JuMP.GenericAffExpr{Float64,VariableRef},2}:
 x  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0
 0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0
 0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0
 0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0
 0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0

[mlubin]

I just updated my example. The issue still exists on Julia 1.0 and JuMP/MOI.