From 4969604734f3e2cb5707e170ae33d8f327cdc1f1 Mon Sep 17 00:00:00 2001 From: Henrique Becker Date: Mon, 4 May 2020 16:00:36 -0300 Subject: [PATCH 1/3] Adds buffering for variables deleted. Gurobi.Optimizer now have fields `next_column` and `columns_deleted_since_last_update`. The behavior now is to not force a update before deleting, but instead: update the list of variables in Julia side but not their column indexes, and save the column indexes of the ones deleted; when updating the model then update the column indexes in Julia side. --- src/MOI_callbacks.jl | 4 ++- src/MOI_wrapper.jl | 81 +++++++++++++++++++++++++++++++------------- 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/src/MOI_callbacks.jl b/src/MOI_callbacks.jl index 5c2a004c..87582c7e 100644 --- a/src/MOI_callbacks.jl +++ b/src/MOI_callbacks.jl @@ -20,7 +20,9 @@ function MOI.set(model::Optimizer, ::CallbackFunction, f::Function) f(cb_data, cb_where) model.callback_state = CB_NONE end) - update_model!(model.inner) + # mark the update as necessary and immediately call for the update + _require_update(model) + _update_if_necessary(model) return end MOI.supports(::Optimizer, ::CallbackFunction) = true diff --git a/src/MOI_wrapper.jl b/src/MOI_wrapper.jl index b4fd3126..c6426398 100644 --- a/src/MOI_wrapper.jl +++ b/src/MOI_wrapper.jl @@ -87,6 +87,24 @@ mutable struct Optimizer <: MOI.AbstractOptimizer # variable type, and the names of SingleVariable-in-Set constraints. variable_info::CleverDicts.CleverDict{MOI.VariableIndex, VariableInfo} + # If you add variables to a model that had variables deleted AND has + # not called `update_model!` since the deletion, then the newly created + # variables may have attributes set, but their column index before the + # call to `update_model!` is different than after the `update_model!`. + # Before the `update_model!` their column is the same as if no variables + # were deleted, after the `update_model!` the columns indexes are + # shifted (by being being subtracted by the number of variables deleted + # with column indexes smaller than them). To control this the two + # fields below are used: + # `next_column`: The column index of the next variable/column added. It is + # updated when variables are added, and when the `_update_if_necessary!` is + # called AND `columns_deleted_since_last_update` is not empty. + # `columns_deleted_since_last_update`: Stores the column indexes of all + # columns that were deleted since the last call to `_update_if_necessary!`, + # after such call the vector is emptied. + next_column::Int + columns_deleted_since_last_update::Vector{Int} + # An index that is incremented for each new constraint (regardless of type). # We can check if a constraint is valid by checking if it is in the correct # xxx_constraint_info. We should _not_ reset this to zero, since then new @@ -135,6 +153,8 @@ mutable struct Optimizer <: MOI.AbstractOptimizer model.silent = false model.params = Dict{String, Any}() model.variable_info = CleverDicts.CleverDict{MOI.VariableIndex, VariableInfo}() + model.next_column = 1 + model.columns_deleted_since_last_update = Int[] model.affine_constraint_info = Dict{Int, ConstraintInfo}() model.quadratic_constraint_info = Dict{Int, ConstraintInfo}() model.sos_constraint_info = Dict{Int, ConstraintInfo}() @@ -169,6 +189,8 @@ function MOI.empty!(model::Optimizer) model.objective_type = SCALAR_AFFINE model.is_feasibility = true empty!(model.variable_info) + model.next_column = 1 + empty!(model.columns_deleted_since_last_update) empty!(model.affine_constraint_info) empty!(model.quadratic_constraint_info) empty!(model.sos_constraint_info) @@ -190,14 +212,16 @@ function MOI.is_empty(model::Optimizer) model.objective_type != SCALAR_AFFINE && return false model.is_feasibility == false && return false !isempty(model.variable_info) && return false - length(model.affine_constraint_info) != 0 && return false - length(model.quadratic_constraint_info) != 0 && return false - length(model.sos_constraint_info) != 0 && return false + !isone(model.next_column) && return false + !isempty(model.columns_deleted_since_last_update) && return false + !isempty(model.affine_constraint_info) && return false + !isempty(model.quadratic_constraint_info) && return false + !isempty(model.sos_constraint_info) && return false model.name_to_variable !== nothing && return false model.name_to_constraint_index !== nothing && return false model.has_unbounded_ray && return false model.has_infeasibility_cert && return false - length(model.callback_variable_primal) != 0 && return false + !isempty(model.callback_variable_primal) && return false model.callback_state != CB_NONE && return false model.has_generic_callback && return false model.lazy_callback !== nothing && return false @@ -223,8 +247,18 @@ Calls `update_model!`, but only if the `model.needs_update` flag is set. """ function _update_if_necessary(model::Optimizer) if model.needs_update + sort!(model.columns_deleted_since_last_update) + for var_info in values(model.variable_info) + var_info.column -= searchsortedlast( + model.columns_deleted_since_last_update, var_info.column + ) + end + model.next_column -= length(model.columns_deleted_since_last_update) + empty!(model.columns_deleted_since_last_update) update_model!(model.inner) model.needs_update = false + else + @assert isempty(model.columns_deleted_since_last_update) end return end @@ -419,6 +453,12 @@ function _info(model::Optimizer, key::MOI.VariableIndex) throw(MOI.InvalidIndex(key)) end +function _get_next_column(model::Optimizer) + old_value = deepcopy(model.next_column) + model.next_column += 1 + return old_value +end + function MOI.add_variable(model::Optimizer) # Initialize `VariableInfo` with a dummy `VariableIndex` and a column, # because we need `add_item` to tell us what the `VariableIndex` is. @@ -428,7 +468,7 @@ function MOI.add_variable(model::Optimizer) info = _info(model, index) # Now, set `.index` and `.column`. info.index = index - info.column = length(model.variable_info) + info.column = _get_next_column(model) add_cvar!(model.inner, 0.0) _require_update(model) return index @@ -436,9 +476,7 @@ end function MOI.add_variables(model::Optimizer, N::Int) add_cvars!(model.inner, zeros(N)) - _require_update(model) indices = Vector{MOI.VariableIndex}(undef, N) - num_variables = length(model.variable_info) for i in 1:N # Initialize `VariableInfo` with a dummy `VariableIndex` and a column, # because we need `add_item` to tell us what the `VariableIndex` is. @@ -448,9 +486,10 @@ function MOI.add_variables(model::Optimizer, N::Int) info = _info(model, index) # Now, set `.index` and `.column`. info.index = index - info.column = num_variables + i + info.column = _get_next_column(model) indices[i] = index end + _require_update(model) return indices end @@ -459,46 +498,38 @@ function MOI.is_valid(model::Optimizer, v::MOI.VariableIndex) end function MOI.delete(model::Optimizer, indices::Vector{<:MOI.VariableIndex}) - _update_if_necessary(model) + #_update_if_necessary(model) info = [_info(model, var_idx) for var_idx in indices] soc_idx = findfirst(e -> e.num_soc_constraints > 0, info) soc_idx !== nothing && throw(MOI.DeleteNotAllowed(indices[soc_idx])) - sorted_del_cols = sort!(collect(i.column for i in info)) - del_vars!(model.inner, convert(Vector{Cint}, sorted_del_cols)) - _require_update(model) + del_cols = collect(i.column for i in info) + del_vars!(model.inner, convert(Vector{Cint}, del_cols)) for var_idx in indices delete!(model.variable_info, var_idx) end - for other_info in values(model.variable_info) - other_info.column -= searchsortedlast( - sorted_del_cols, other_info.column - ) - end + append!(model.columns_deleted_since_last_update, del_cols) model.name_to_variable = nothing # We throw away name_to_constraint_index so we will rebuild SingleVariable # constraint names without v. model.name_to_constraint_index = nothing + _require_update(model) return end function MOI.delete(model::Optimizer, v::MOI.VariableIndex) - _update_if_necessary(model) + #_update_if_necessary(model) info = _info(model, v) if info.num_soc_constraints > 0 throw(MOI.DeleteNotAllowed(v)) end + push!(model.columns_deleted_since_last_update, info.column) del_vars!(model.inner, Cint[info.column]) - _require_update(model) delete!(model.variable_info, v) - for other_info in values(model.variable_info) - if other_info.column > info.column - other_info.column -= 1 - end - end model.name_to_variable = nothing # We throw away name_to_constraint_index so we will rebuild SingleVariable # constraint names without v. model.name_to_constraint_index = nothing + _require_update(model) return end @@ -622,6 +653,8 @@ function MOI.set( column = _info(model, term.variable_index).column obj[column] += term.coefficient end + # NOTE: variables added may be referred before a `_update_if_necessary` + # what is the problem we try to prevent below? # This update is needed because we might have added some variables. _update_if_necessary(model) set_dblattrarray!(model.inner, "Obj", 1, num_vars, obj) From 77cf95ea9476b4d53cf8e6000c7c4a77cddcbea7 Mon Sep 17 00:00:00 2001 From: Henrique Becker Date: Mon, 4 May 2020 19:33:39 -0300 Subject: [PATCH 2/3] Changed `_get_next_column` and added test. --- src/MOI_wrapper.jl | 3 +-- test/MOI/MOI_wrapper.jl | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/MOI_wrapper.jl b/src/MOI_wrapper.jl index c6426398..898f3b6c 100644 --- a/src/MOI_wrapper.jl +++ b/src/MOI_wrapper.jl @@ -454,9 +454,8 @@ function _info(model::Optimizer, key::MOI.VariableIndex) end function _get_next_column(model::Optimizer) - old_value = deepcopy(model.next_column) model.next_column += 1 - return old_value + return model.next_column - 1 end function MOI.add_variable(model::Optimizer) diff --git a/test/MOI/MOI_wrapper.jl b/test/MOI/MOI_wrapper.jl index 1392675e..9e263c3e 100644 --- a/test/MOI/MOI_wrapper.jl +++ b/test/MOI/MOI_wrapper.jl @@ -492,6 +492,37 @@ end }())) end +@testset "Buffered deletion test" begin + # Check if the names of variables (could be any attribute that is not + # cached as the bounds are) are right if we remove some variables and + # add new ones without updating the model (and that they stay correct + # after updating the model, here done by `MOI.optimize!`, as the update + # needs to be done inside MOI code, that is aware of the lazy updates). + model = Gurobi.Optimizer(GUROBI_ENV) + vars = MOI.add_variables(model, 3) + var_names = ["x_1", "x_2", "x_3"] + MOI.set.(model, MOI.VariableName(), vars, var_names) + @test all(MOI.is_valid.(model, vars)) + @test MOI.get.(model, MOI.VariableName(), vars) == var_names + MOI.delete(model, vars[2:3]) + @test !MOI.is_valid(model, vars[2]) + @test !MOI.is_valid(model, vars[3]) + fourth_var = MOI.add_variable(model) + MOI.set(model, MOI.VariableName(), fourth_var, "x_4") + # Check before updating the model and after the delete+insert. + @test MOI.is_valid(model, vars[1]) + @test "x_1" == MOI.get(model, MOI.VariableName(), vars[1]) + @test MOI.is_valid(model, fourth_var) + @test "x_4" == MOI.get(model, MOI.VariableName(), fourth_var) + # Then optimize to force an update. + MOI.optimize!(model) + # And check again. + @test MOI.is_valid(model, vars[1]) + @test "x_1" == MOI.get(model, MOI.VariableName(), vars[1]) + @test MOI.is_valid(model, fourth_var) + @test "x_4" == MOI.get(model, MOI.VariableName(), fourth_var) +end + @testset "Extra name tests" begin model = Gurobi.Optimizer(GUROBI_ENV) @testset "Variables" begin From d31df42b90068b3e5be1b56d08974a429ed7f1b2 Mon Sep 17 00:00:00 2001 From: Henrique Becker Date: Mon, 4 May 2020 22:19:44 -0300 Subject: [PATCH 3/3] Changed test to a variable attribute that is not cached. --- test/MOI/MOI_wrapper.jl | 60 ++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/test/MOI/MOI_wrapper.jl b/test/MOI/MOI_wrapper.jl index 9e263c3e..372ff2c7 100644 --- a/test/MOI/MOI_wrapper.jl +++ b/test/MOI/MOI_wrapper.jl @@ -493,36 +493,42 @@ end end @testset "Buffered deletion test" begin - # Check if the names of variables (could be any attribute that is not - # cached as the bounds are) are right if we remove some variables and - # add new ones without updating the model (and that they stay correct - # after updating the model, here done by `MOI.optimize!`, as the update - # needs to be done inside MOI code, that is aware of the lazy updates). + # Check if the VarHintVal of variables (could be any attribute that is not + # cached, the most common as: the bounds, name, and the start, are all + # cached) are right if we remove some variables and add new ones without + # updating the model (and that they stay correct after updating the model, + # here done by `MOI.optimize!`, as the update needs to be done inside MOI + # code, that is aware of the lazy updates). model = Gurobi.Optimizer(GUROBI_ENV) - vars = MOI.add_variables(model, 3) - var_names = ["x_1", "x_2", "x_3"] - MOI.set.(model, MOI.VariableName(), vars, var_names) - @test all(MOI.is_valid.(model, vars)) - @test MOI.get.(model, MOI.VariableName(), vars) == var_names - MOI.delete(model, vars[2:3]) - @test !MOI.is_valid(model, vars[2]) - @test !MOI.is_valid(model, vars[3]) - fourth_var = MOI.add_variable(model) - MOI.set(model, MOI.VariableName(), fourth_var, "x_4") - # Check before updating the model and after the delete+insert. - @test MOI.is_valid(model, vars[1]) - @test "x_1" == MOI.get(model, MOI.VariableName(), vars[1]) - @test MOI.is_valid(model, fourth_var) - @test "x_4" == MOI.get(model, MOI.VariableName(), fourth_var) - # Then optimize to force an update. - MOI.optimize!(model) - # And check again. - @test MOI.is_valid(model, vars[1]) - @test "x_1" == MOI.get(model, MOI.VariableName(), vars[1]) - @test MOI.is_valid(model, fourth_var) - @test "x_4" == MOI.get(model, MOI.VariableName(), fourth_var) + vars = MOI.add_variables(model, 3) + vars_obj = [7.0, 42.0, -0.5] + obj_attr = Gurobi.VariableAttribute("Obj") + MOI.set.(model, obj_attr, vars, vars_obj) + @test all(MOI.is_valid.(model, vars)) + @test MOI.get.(model, obj_attr, vars) == vars_obj + MOI.delete(model, vars[[1, 3]]) + @test !MOI.is_valid(model, vars[1]) + @test !MOI.is_valid(model, vars[3]) + fourth_var = MOI.add_variable(model) + fourth_var_obj = -77.0 + MOI.set(model, obj_attr, fourth_var, fourth_var_obj) + # Check before updating the model and after the delete+insert. + @test !MOI.is_valid(model, vars[1]) + @test !MOI.is_valid(model, vars[3]) + @test MOI.is_valid(model, vars[2]) + @test vars_obj[2] == MOI.get(model, obj_attr, vars[2]) + @test MOI.is_valid(model, fourth_var) + @test fourth_var_obj == MOI.get(model, obj_attr, fourth_var) + # Then optimize to force an update. + MOI.optimize!(model) + # And check again. + @test MOI.is_valid(model, vars[2]) + @test vars_obj[2] == MOI.get(model, obj_attr, vars[2]) + @test MOI.is_valid(model, fourth_var) + @test fourth_var_obj == MOI.get(model, obj_attr, fourth_var) end + @testset "Extra name tests" begin model = Gurobi.Optimizer(GUROBI_ENV) @testset "Variables" begin