sum comparison parses wrongly without error

[Wikunia]

I'm currently thinking about allowing comparisons inside a sum for my ConstraintSolver.jl package. (See: Wikunia/ConstraintSolver.jl#254)

Unfortunately I encountered the following problem: When combining comparisons inside the sum constraint with && i.e

model = Model()
@variable(model, x[1:5], Bin)
@constraint(model, sum(x[i] == 1 && x[i+1] == 1 for i=1:4) <= 1)

it currently gets parsed as 0 <= 1 instead of throwing an error as it does for things like:

@constraint(model, sum(x[i] == 1 for i=1:4) <= 1)

It's quite unlikely that someone encounters this but would be nice if it throws an error.

[blegat]

The error is throw in
https://github.com/jump-dev/MutableArithmetics.jl/blob/595460dd664380e7668624fccb4f26a637fc1af6/src/rewrite.jl#L628-L629
It might be complicted to throw an error at parse time for the above constraint too.
One option might be to implement ==(::VariableRef, ::Number) and throw an error. Currently, it falls back to === which returns false and often silently leads to unexpected behaviors.

[odow]

Mostly because of this (simplified, lines removed):

julia> @macroexpand @constraint(model, sum(x == i && x == i for i = 1:2) <= 1)
ss0 = JuMP.MutableArithmetics.Zero()
for i = 1:2
    ss0 = JuMP.MutableArithmetics.operate!(
        JuMP.MutableArithmetics.add_mul, 
        ss0, 
        x == i && x == i
    )
end

It seems reasonable to define ==(::VariableRef, ::Number) and ==(::Number, ::VariableRef), because this does frequently cause issues.

It doesn't help @Wikunia though, but that is because @constraint can't create nonlinear expressions like this.

@blegat, the error you're thinking of is

julia> @constraint(model, sum(x == i for i = 1:2) <= 1)
ERROR: LoadError: Unexpected comparison in expression `x == i`.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] _rewrite(::Bool, ::Bool, ::Expr, ::Symbol, ::Array{Any,1}, ::Array{Any,1}, ::Symbol) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:629
 [3] #8 at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:227 [inlined]
 [4] rewrite_generator(::Expr, ::MutableArithmetics.var"#8#10"{Symbol,Bool,Bool,Array{Any,1},Array{Any,1}}) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:143
 [5] rewrite_generator(::Expr, ::MutableArithmetics.var"#8#10"{Symbol,Bool,Bool,Array{Any,1},Array{Any,1}}) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:168
 [6] (::MutableArithmetics.var"#7#9"{Bool,Bool,Expr,Array{Any,1},Array{Any,1},Symbol})(::Symbol) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:225
 [7] _start_summing(::Nothing, ::MutableArithmetics.var"#7#9"{Bool,Bool,Expr,Array{Any,1},Array{Any,1},Symbol}) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:332
 [8] _parse_generator_sum at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:222 [inlined]
 [9] _parse_generator(::Bool, ::Bool, ::Expr, ::Nothing, ::Array{Any,1}, ::Array{Any,1}, ::Symbol) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:196
 [10] _rewrite(::Bool, ::Bool, ::Expr, ::Nothing, ::Array{Any,1}, ::Array{Any,1}, ::Symbol) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:611
 [11] _rewrite(::Bool, ::Bool, ::Expr, ::Nothing, ::Array{Any,1}, ::Array{Any,1}) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:403
 [12] _rewrite(::Bool, ::Bool, ::Expr, ::Nothing, ::Array{Any,1}, ::Array{Any,1}, ::Symbol) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:429
 [13] _rewrite at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:403 [inlined]
 [14] rewrite_and_return(::Expr) at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:272
 [15] rewrite at /Users/oscar/.julia/packages/MutableArithmetics/bPWR4/src/rewrite.jl:261 [inlined]
 [16] parse_one_operator_constraint(::Function, ::Bool, ::Val{:<=}, ::Expr, ::Int64) at /Users/oscar/.julia/dev/JuMP/src/macros.jl:198
 [17] parse_constraint(::Function, ::Symbol, ::Expr, ::Int64) at /Users/oscar/.julia/dev/JuMP/src/macros.jl:211
 [18] parse_constraint_head(::Function, ::Val{:call}, ::Symbol, ::Expr, ::Int64) at /Users/oscar/.julia/dev/JuMP/src/macros.jl:206
 [19] parse_constraint_expr at /Users/oscar/.julia/dev/JuMP/src/macros.jl:203 [inlined]
 [20] _constraint_macro(::Tuple{Symbol,Expr}, ::Symbol, ::typeof(parse_constraint_expr), ::LineNumberNode) at /Users/oscar/.julia/dev/JuMP/src/macros.jl:438
 [21] @constraint(::LineNumberNode, ::Module, ::Vararg{Any,N} where N) at /Users/oscar/.julia/dev/JuMP/src/macros.jl:521
in expression starting at REPL[18]:1

[odow]

#2729 adds a better error message for inequality comparisons. But I don't think we can throw a better error for x == i because there are legitimate use-cases for returning false. e.g., Base Julia uses x == 0 a lot instead of iszero(x).

[odow]

Closing because I don't think we can easily support this. We now throw better error messages for <, <=, >, and >=. But we need to keep x == i for a variety of reasonable use-cases.

The correct way to implement this is to overload parse_constraint_call and define syntax like

@constraint(
    model, 
    BooleanConstraint(sum(x == i && x == i for i = 1:2) <= 1),
)