Wikunia / ConstraintSolver.jl

ConstraintSolver in Julia: Blog posts ->
https://opensourc.es/blog/constraint-solver-1
MIT License
136 stars 14 forks source link

CS.TableSet() requires only VariableRef:s #235

Closed hakank closed 3 years ago

hakank commented 3 years ago

I have the following model, playing with modulo (again).

The error is that y is a Int64 of 4 but CS.TableSet() requires that all variables in the set must be VariableRef:s.

using ConstraintSolver, JuMP
const CS = ConstraintSolver

# 
# modulo(model,x,y,z)
# 
# ensures that z = x mod y 
#
function modulo(model, x, y, z)
    lbx = has_lower_bound(x) ? round(Int, JuMP.lower_bound(x)) : x
    ubx = has_upper_bound(x) ? round(Int, JuMP.upper_bound(x)) : x
    lby = has_lower_bound(y) ? round(Int, JuMP.lower_bound(y)) : y
    uby = has_upper_bound(y) ? round(Int, JuMP.upper_bound(y)) : y

    table = resize_matrix([ [i,j,i % j] for i in lbx:ubx, j in lby:uby if j != 0])
    @constraint(model, [x, y, z] in CS.TableSet(table))
end   

model = Model(optimizer_with_attributes(CS.Optimizer,"logging"=>[]))

@variable(model,1 <= x <= 10, Int)
# @variable(model,1 <= y <= 10, Int) # This works
y = 4 # Int64
# @variable(model,4 <= y <= 4, Int) # This works
@variable(model,0 <= z <= 10, Int)

@constraint(model, x!=y)

modulo(model,x,y,z)

optimize!(model)
status = JuMP.termination_status(model)
println("status: $status")
if status == MOI.OPTIMAL
    x_val = convert.(Integer,JuMP.value.(x))
    # y_val = convert.(Integer,JuMP.value.(y))
    z_val = convert.(Integer,JuMP.value.(z))
    # println("x:$x_val y_val:$y_val z:$z_val")
    println("x:$x_val y_val:$y z:$z_val")
else 
    println("No solution")
end 

It yields the following non-instructive error message:

ERROR: Each variable must be an integer and bounded. Currently the variable index 3 doesn't fulfill this requirements.

The model works if y was instead was defined like this:

@variable(model,4 <= y <= 4, Int)

I have some questions wishes regarding this: 1) Would it be possible that CS.TableSet() also can handle Int64 and not require VariableRef? 2) It would be great if the error message would be a little more instructive. 3) What is the principled way of analysing these kind of problems, i.e. see what index 3 refers to?

The real model is a my standard explorations of ISBN: http://hakank.org/julia/constraints/isbn.jl where the problematic variable is the constant 10 (defined as ten) . And I have now also added variants to the original modulo definition which catch if some of the variables (x, y, or z) are Int:s; this is in http://hakank.org/julia/constraints/constraints_utils.jl .

function modulo(model, x::Int, y, z)    
    println("modulo(model, $x, $y, $z) typeof x:$(typeof(x)) y:$(typeof(y)) z:$(typeof(z))")
    error("modulo/3 has a variable (x) that is not a decision value. CS.TableSet() requires that all values are VariableRef. ")
end

function modulo(model, x, y::Int, z)    
    println("modulo(model, $x, $y, $z) typeof x:$(typeof(x)) y:$(typeof(y)) z:$(typeof(z))")
    error("modulo/3 has a variable (y) that is not a decision value. CS.TableSet() requires that all values are VariableRef. ")
end

function modulo(model, x, y, z::Int)    
    println("modulo(model, $x, $y, $z) typeof x:$(typeof(x)) y:$(typeof(y)) z:$(typeof(z))")
    error("modulo/3 has a variable (z) that is not a decision value. CS.TableSet() requires that all values are VariableRef. ")
end
Wikunia commented 3 years ago

Hey Hakan, happy new year :tada: Thanks for opening the issue.

  1. I haven't thought about allowing integers as they can easily be removed but I now think it makes sense to just allow it for convenience. Will have a look into that.
  2. Unfortunately I'm not sure whether this is directly possible as I'm not sure I have access to the name of the variable by its index. Will ask around and will add this if possible.
  3. The problem is really that the underlying layer is responsible for the mapping and the mapping can be different when adding new constraints.
hakank commented 3 years ago

Thanks for the answers, Ole.

And - of course - a Happy New Year to you too!

Wikunia commented 3 years ago

Currently it actually fails because has_lower_bound isn't defined for Int, right?

hakank commented 3 years ago

Ah, yes. I added the lower/upper bounds for testing and forgot. Sorry if that was confusing.

Wikunia commented 3 years ago

I now see the actual problem: [x, y, z] in CS.TableSet is transformed into a set of variables in JuMP as the function and the information about the value y = 4 is lost. This can't really be fixed on my side.

You might want to change your modulo function to:

function modulo(model, x, y, z)
    if x isa Integer
        x = @variable(model, lower_bound=x, upper_bound=x, integer=true)
    end
    if y isa Integer
        y = @variable(model, lower_bound=y, upper_bound=y, integer=true)
    end
    lbx = round(Int, JuMP.lower_bound(x))
    ubx = round(Int, JuMP.upper_bound(x))
    lby = round(Int, JuMP.lower_bound(y))
    uby = round(Int, JuMP.upper_bound(y))

    table = resize_matrix([ [i,j,i % j] for i in lbx:ubx, j in lby:uby if j != 0])
    @constraint(model, [x, y, z] in CS.TableSet(table))
end

then it should work with integers and there isn't really a downside in adding a new fixed variable here.

hakank commented 3 years ago

Thanks, that's a great solution! (I didn't think of the isa Integer check.)

hakank commented 3 years ago

This is probably related so I don't start a new issue.

Here's a model of n-queens using the standard all_different approach. But it don't work since CS.TableSet() throws the error (see below for the complete stacktrace):

Each variable must be an integer and bounded. Currently the variable index 9 doesn't fulfill this requirements.
using ConstraintSolver, JuMP
const CS = ConstraintSolver

function nqueens(n=8)
    model = Model(optimizer_with_attributes(CS.Optimizer))

    @variable(model, 1 <= x[1:n] <= n, Int)
    @constraint(model, x in CS.AllDifferentSet())
    @constraint(model, [x[i] + i for i in 1:n] in CS.AllDifferentSet())
    @constraint(model, [x[i] - i for i in 1:n] in CS.AllDifferentSet())

    optimize!(model)

    status = JuMP.termination_status(model)

    if status == MOI.OPTIMAL
        num_sols = MOI.get(model, MOI.ResultCount())
        if print_solutions
            for sol in 1:num_sols
                x_val = convert.(Integer,JuMP.value.(x; result=sol))
                println("x:$x_val")
            end
        end
    end
end

@time nqueens(8)

The complete error is

ERROR: Each variable must be an integer and bounded. Currently the variable index 9 doesn't fulfill this requirements.
Stacktrace:
  [1] check_var_bounds(model::ConstraintSolver.Optimizer)
    @ ConstraintSolver ~/.julia/packages/ConstraintSolver/FrWVK/src/MOI_wrapper/variables.jl:36
  [2] optimize!(model::ConstraintSolver.Optimizer)
    @ ConstraintSolver ~/.julia/packages/ConstraintSolver/FrWVK/src/MOI_wrapper/MOI_wrapper.jl:142
  [3] optimize!(b::MathOptInterface.Bridges.LazyBridgeOptimizer{ConstraintSolver.Optimizer})
    @ MathOptInterface.Bridges ~/.julia/packages/MathOptInterface/ZJFKw/src/Bridges/bridge_optimizer.jl:264
  [4] optimize!(m::MathOptInterface.Utilities.CachingOptimizer{MathOptInterface.AbstractOptimizer, MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/ZJFKw/src/Utilities/cachingoptimizer.jl:215
  [5] optimize!(model::Model, optimizer_factory::Nothing; bridge_constraints::Bool, ignore_optimize_hook::Bool, kwargs::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ JuMP ~/.julia/packages/JuMP/e0Uc2/src/optimizer_interface.jl:130
  [6] optimize!
    @ ~/.julia/packages/JuMP/e0Uc2/src/optimizer_interface.jl:106 [inlined]
  [7] nqueens(n::Int64)
    @ Main ~/julia/constraints/nqueens_simple.jl:12
  [8] top-level scope
    @ ./timing.jl:206
  [9] include(fname::String)
    @ Base.MainInclude ./client.jl:444
 [10] top-level scope
    @ ./timing.jl:206 [inlined]
 [11] top-level scope
    @ ./REPL[13]:0
 [12] eval
    @ ./boot.jl:360 [inlined]

For this specific problem, it's quite easily fixed by adding two extra arrays (t1 and t2), but it would be great if the "direct" approach was possible:

# ...
   @variable(model, 1 <= x[1:n] <= n, Int)
    @variable(model, 0 <= t1[1:n] <= n*2, Int)
    @variable(model, -n <= t2[1:n] <= n, Int)

    @constraint(model, x in CS.AllDifferentSet())
    for i in 1:n
        @constraint(model, x[i] + i == t1[i])    
        @constraint(model, x[i] - i == t2[i])    
    end
    @constraint(model, t1 in CS.AllDifferentSet())
    @constraint(model, t2 in CS.AllDifferentSet())
# ...
Wikunia commented 3 years ago

Hmm a good one :smile:

I think this needs to be tackled similar to the element constraint such that the variables are generated on the fly. For that I need to do some more work on the indicator and reified constraints as they currently limit the possibility of creating any new variables/constraints.

Will think more about it but I think this will take quite a bit time to do it right.