Subtle trap with semicolon syntax

[mlubin]

julia> m = Model();

julia> @variable(m, x[i=1:4; iseven(i)])
JuMP.Containers.SparseAxisArray{VariableRef,1,Tuple{Int64}} with 2 entries:
  [4]  =  x[4]
  [2]  =  x[2]

julia> @variable(m, y[i=1:4, iseven(i)])
JuMP.Containers.SparseAxisArray{VariableRef,2,Tuple{Int64,Bool}} with 4 entries:
  [1, false]  =  y[1,false]
  [3, false]  =  y[3,false]
  [4, true ]  =  y[4,true]
  [2, true ]  =  y[2,true]

I'm not sure how to fix this, but it's unfortunate that a one-character change that's hard to spot can have this subtle effect.

[odow]

I didn't even realize this would work.

The only way I see to fix this is to revive our old discussion with something along the lines of:

@variable(model, x[i=1:4])  # x[1]
@variable(model, x[i=1:4, j=i:5])  # x[1, 2]
@variable(model, x[i for i=1:4 if iseven(i)])  # x[2]

@variable(model, x[(i, j) for i=1:4, j=2:3 if iseven(i+ j)])  # x[(2, 2)] or x[2, 2]?

For backward compatibility, the last one could just create a Dict{Tuple{Int, Int}, VariableRef}, which would clarify slicing issues, etc.

[blegat]

We can also replace the ; syntax by a if syntax without changing anything else.
The second example would still work though. We require that it is an iterator and iseven(i) acts as a one element iterator so it's perfectly valid. We could make an exception and disallow the iterator being Bool. It would make sense as the user can use Vector{Bool} instead

[mlubin]

We can also replace the ; syntax by a if syntax without changing anything else.

How would we do that? @variable(m, x[i=1:4 if iseven(i)]) doesn't parse.

Printing a warning if index sets are scalars might make sense. It probably will catch a number of subtle bugs. On the other hand, it may annoy people if we break from Julia's rules for what's iterable.

[blegat]

The user could do Model(disable_scalar_iterator_warning=false) to allow the user to disable this. A common mistake I do is to write length(a) instead of 1:length(a)

[odow]

This came up on Discourse: https://discourse.julialang.org/t/error-syntax-function-argument-names-not-unique-around/53944

All options are pretty unhelpful, and could do with a better error message.

model = Model()
@variable(model, x)

julia> @constraint(model, [s=1:2, s = 1], x <= s)
ERROR: syntax: function argument names not unique around REPL[15]:1
Stacktrace:
 [1] top-level scope at REPL[15]:1

julia> @constraint(model, [s=1:2, s == 1], x <= s)
JuMP.Containers.SparseAxisArray{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},2,Tuple{Int64,Bool}} with 2 entries:
  [2, false]  =  x ≤ 2.0
  [1, true ]  =  x ≤ 1.0

julia> @constraint(model, [s=1:2; s = 1], x <= s)
ERROR: TypeError: non-boolean (Int64) used in boolean context
Stacktrace:
 [1] first_iterate at /Users/oscar/.julia/packages/JuMP/qhoVb/src/Containers/nested_iterator.jl:50 [inlined]
 [2] next_iterate at /Users/oscar/.julia/packages/JuMP/qhoVb/src/Containers/nested_iterator.jl:41 [inlined]
 [3] first_iterate at /Users/oscar/.julia/packages/JuMP/qhoVb/src/Containers/nested_iterator.jl:58 [inlined]
 [4] iterate at /Users/oscar/.julia/packages/JuMP/qhoVb/src/Containers/nested_iterator.jl:69 [inlined]
 [5] iterate at ./generator.jl:44 [inlined]
 [6] grow_to!(::JuMP.Containers.NoDuplicateDict{Any,Any}, ::Base.Generator{JuMP.Containers.NestedIterator{Tuple{var"#47#50"},var"#48#51"},JuMP.Containers.var"#30#31"{var"#46#49"}}) at ./dict.jl:139
 [7] dict_with_eltype at ./abstractdict.jl:540 [inlined]
 [8] JuMP.Containers.NoDuplicateDict(::Base.Generator{JuMP.Containers.NestedIterator{Tuple{var"#47#50"},var"#48#51"},JuMP.Containers.var"#30#31"{var"#46#49"}}) at /Users/oscar/.julia/packages/JuMP/qhoVb/src/Containers/no_duplicate_dict.jl:39
 [9] container(::Function, ::JuMP.Containers.NestedIterator{Tuple{var"#47#50"},var"#48#51"}, ::Type{JuMP.Containers.SparseAxisArray}) at /Users/oscar/.julia/packages/JuMP/qhoVb/src/Containers/container.jl:96
 [10] container(::Function, ::JuMP.Containers.NestedIterator{Tuple{var"#47#50"},var"#48#51"}) at /Users/oscar/.julia/packages/JuMP/qhoVb/src/Containers/container.jl:65
 [11] top-level scope at REPL[17]:1

julia> @constraint(model, [s=1:2; s == 1], x <= s)
JuMP.Containers.SparseAxisArray{ConstraintRef{Model,MathOptInterface.ConstraintIndex{MathOptInterface.ScalarAffineFunction{Float64},MathOptInterface.LessThan{Float64}},ScalarShape},1,Tuple{Int64}} with 1 entry:
  [1]  =  x ≤ 1.0

[odow]

We now have:

julia> @constraint(model, [s=1:2, s = 1], x <= s)
ERROR: LoadError: At REPL[64]:1: `@constraint(model, [s = 1:2, s = 1], x <= s)`: The index s appears more than once. The index associated with each set must be unique.
Stacktrace:
 [1] error(::String, ::String)
   @ Base ./error.jl:42
 [2] _macro_error(macroname::Symbol, args::Tuple{Symbol, Expr, Expr}, source::LineNumberNode, str::String)
   @ JuMP ~/.julia/dev/JuMP/src/macros.jl:1558
 [3] (::JuMP.var"#_error#92"{Tuple{Symbol, Expr, Expr}, Symbol, LineNumberNode})(str::String)
   @ JuMP ~/.julia/dev/JuMP/src/macros.jl:657
 [4] _parse_ref_sets(_error::JuMP.var"#_error#92"{Tuple{Symbol, Expr, Expr}, Symbol, LineNumberNode}, expr::Expr)
   @ JuMP.Containers ~/.julia/dev/JuMP/src/Containers/macro.jl:134
 [5] _build_ref_sets(_error::JuMP.var"#_error#92"{Tuple{Symbol, Expr, Expr}, Symbol, LineNumberNode}, expr::Expr)
   @ JuMP.Containers ~/.julia/dev/JuMP/src/Containers/macro.jl:158
 [6] _constraint_macro(args::Tuple{Symbol, Expr, Expr}, macro_name::Symbol, parsefun::typeof(parse_constraint_expr), source::LineNumberNode)
   @ JuMP ~/.julia/dev/JuMP/src/macros.jl:716
 [7] var"@constraint"(__source__::LineNumberNode, __module__::Module, args::Vararg{Any, N} where N)
   @ JuMP ~/.julia/dev/JuMP/src/macros.jl:829
in expression starting at REPL[64]:1

And I pushed #2730 which gives us

julia> @variable(model, x[1:2, t=1])
┌ Warning: Axis contains one element: 1. If intended, you can safely ignore this warning. To explicitly pass the axis with one element, pass `[1]` instead of `1`.
└ @ JuMP.Containers ~/.julia/dev/JuMP/src/Containers/DenseAxisArray.jl:153
2-dimensional DenseAxisArray{VariableRef,2,...} with index sets:
    Dimension 1, Base.OneTo(2)
    Dimension 2, fill(1)
And data, a 2×1 Matrix{VariableRef}:
 x[1,1]
 x[2,1]

But I don't know what else we can do here. There are lots of reasons why someone would want to do either of @mlubin's original suggestions, and we can't distinguish their intent.

[odow]

Closing because I don't know what else we can do here without massively breaking JuMP syntax (which we won't be doing). The syntax isn't perfect, but its a local optima.

If any future reader stumbles across this and has a suggestion, please leave a comment.