Merge pull request #983 from JuliaOpt/bl/delete_vector_variables

Faster deletion of vector of variables

Author: blegat

docs/src/apireference.md

@@ -962,6 +962,7 @@ The following utilities are available for functions:
 Utilities.eval_variables
 Utilities.map_indices
 Utilities.substitute_variables
+Utilities.filter_variables
 Utilities.remove_variable
 Utilities.all_coefficients
 Utilities.unsafe_add

src/Utilities/functions.jl

@@ -618,38 +618,55 @@ function test_models_equal(model1::MOI.ModelLike, model2::MOI.ModelLike, variabl
 end
 
 
-_hasvar(v::VI, vi::Vector{VI}) = v in vi
-_hasvar(v::VI, vi::VI) = v == vi
-_hasvar(t::MOI.ScalarAffineTerm, vi) = _hasvar(t.variable_index, vi)
-_hasvar(t::MOI.ScalarQuadraticTerm, vi) = _hasvar(t.variable_index_1, vi) || _hasvar(t.variable_index_2, vi)
-_hasvar(t::Union{MOI.VectorAffineTerm, MOI.VectorQuadraticTerm}, vi) = _hasvar(t.scalar_term, vi)
+_keep_all(keep::Function, v::MOI.VariableIndex) = keep(v)
+_keep_all(keep::Function, t::MOI.ScalarAffineTerm) = keep(t.variable_index)
+_keep_all(keep::Function, t::MOI.ScalarQuadraticTerm) = keep(t.variable_index_1) && keep(t.variable_index_2)
+_keep_all(keep::Function, t::Union{MOI.VectorAffineTerm, MOI.VectorQuadraticTerm}) = _keep_all(keep, t.scalar_term)
 # Removes terms or variables in `vis_or_terms` that contains the variable of index `vi`
-function _rmvar(vis_or_terms::Vector, vi)
-    return vis_or_terms[.!_hasvar.(vis_or_terms, Ref(vi))]
+function _filter_variables(keep::Function, variables_or_terms::Vector)
+    return filter(el -> _keep_all(keep, el), variables_or_terms)
 end
 
 """
-    remove_variable(f::AbstractFunction, vi::VariableIndex)
+    filter_variables(keep::Function, f::AbstractFunction)
 
-Return a new function `f` with the variable vi removed.
+Return a new function `f` with the variable `vi` such that `!keep(vi)` removed.
 """
-function remove_variable end
-function remove_variable(f::MOI.SingleVariable, vi::MOI.VariableIndex)
-    if f.variable == vi
+function filter_variables end
+function filter_variables(keep::Function, f::MOI.SingleVariable)
+    if !keep(f.variable)
         error("Cannot remove variable from a `SingleVariable` function of the",
               " same variable.")
     end
     return f
 end
-function remove_variable(f::VVF, vi)
-    VVF(_rmvar(f.variables, vi))
+function filter_variables(keep::Function, f::MOI.VectorOfVariables)
+    return MOI.VectorOfVariables(_filter_variables(keep, f.variables))
+end
+function filter_variables(
+    keep::Function, f::Union{MOI.ScalarAffineFunction, MOI.VectorAffineFunction})
+    return typeof(f)(_filter_variables(keep, f.terms), MOI.constant(f))
 end
-function remove_variable(f::Union{SAF, VAF}, vi)
-    typeof(f)(_rmvar(f.terms, vi), MOI.constant(f))
+function filter_variables(
+    keep, f::Union{MOI.ScalarQuadraticFunction, MOI.VectorQuadraticFunction})
+    return typeof(f)(
+        _filter_variables(keep, f.affine_terms),
+        _filter_variables(keep, f.quadratic_terms),
+        MOI.constant(f))
+end
+
+"""
+    remove_variable(f::AbstractFunction, vi::VariableIndex)
+
+Return a new function `f` with the variable vi removed.
+"""
+function remove_variable(f::MOI.AbstractFunction, vi::MOI.VariableIndex)
+    return filter_variables(v -> v != vi, f)
 end
-function remove_variable(f::Union{SQF, VQF}, vi)
-    terms = _rmvar.((f.affine_terms, f.quadratic_terms), Ref(vi))
-    typeof(f)(terms..., MOI.constant(f))
+function remove_variable(f::MOI.AbstractFunction, vis::Vector{MOI.VariableIndex})
+    # Create a `Set` to test membership in `vis` in O(1).
+    set = Set(vis)
+    return filter_variables(vi -> !(vi in set), f)
 end
 
 """
@@ -671,7 +688,7 @@ function modify_function(f::VQF, change::MOI.VectorConstantChange)
 end
 function _modifycoefficient(terms::Vector{<:MOI.ScalarAffineTerm}, variable::VI, new_coefficient)
     terms = copy(terms)
-    i = something(findfirst(t -> _hasvar(t, variable), terms), 0)
+    i = something(findfirst(t -> t.variable_index == variable, terms), 0)
     if iszero(i)
         # The variable was not already in the function
         if !iszero(new_coefficient)
@@ -701,7 +718,7 @@ function _modifycoefficients(n, terms::Vector{<:MOI.VectorAffineTerm}, variable:
     rowmap = Dict(c[1]=>i for (i,c) in enumerate(new_coefficients))
     del = Int[]
     for i in 1:length(terms)
-        if _hasvar(terms[i], variable)
+        if terms[i].scalar_term.variable_index == variable
             row = terms[i].output_index
             j = Base.get(rowmap, row, 0)
             if !iszero(j) # If it is zero, it means that the row should not be changed

src/Utilities/model.jl

@@ -127,12 +127,27 @@ function remove_variable(f::MOI.VectorOfVariables, s, vi::VI)
     end
     return g, t
 end
-function _remove_variable(constrs::Vector, vi::VI)
+function _remove_variable(constrs::Vector{<:ConstraintEntry}, vi::VI)
     for i in eachindex(constrs)
         ci, f, s = constrs[i]
         constrs[i] = (ci, remove_variable(f, s, vi)...)
     end
-    return CI{MOI.SingleVariable}[]
+end
+filter_variables(keep::Function, f, s) = filter_variables(keep, f), s
+function filter_variables(keep::Function, f::MOI.VectorOfVariables, s)
+    g = filter_variables(keep, f)
+    if length(g.variables) != length(f.variables)
+        t = MOI.update_dimension(s, length(g.variables))
+    else
+        t = s
+    end
+    return g, t
+end
+function _filter_variables(keep::Function, constrs::Vector{<:ConstraintEntry})
+    for i in eachindex(constrs)
+        ci, f, s = constrs[i]
+        constrs[i] = (ci, filter_variables(keep, f, s)...)
+    end
 end
 function _vector_of_variables_with(::Vector, ::Union{VI, MOI.Vector{VI}})
     return CI{MOI.VectorOfVariables}[]
@@ -172,19 +187,15 @@ function _vector_of_variables_with(
     end
     return rm
 end
-function MOI.delete(model::AbstractModel{T}, vi::VI) where T
+function _delete_variable(model::AbstractModel{T}, vi::MOI.VariableIndex) where T
     MOI.throw_if_not_valid(model, vi)
-    model.objective = remove_variable(model.objective, vi)
     # If a variable is removed, the `VectorOfVariables` constraints using this
     # variable only need to be removed too. `vov_to_remove` is the list of
     # indices of the `VectorOfVariables` constraints of `vi`.
     vov_to_remove = broadcastvcat(constrs -> _vector_of_variables_with(constrs, vi), model)
     for ci in vov_to_remove
         MOI.delete(model, ci)
     end
-    # `VectorOfVariables` constraints with sets not supporting dimension update
-    # were either deleted or an error was thrown. The rest is modified now.
-    broadcastcall(constrs -> _remove_variable(constrs, vi), model)
     model.single_variable_mask[vi.value] = 0x0
     if model.variable_indices === nothing
         model.variable_indices = Set(MOI.get(model,
@@ -203,16 +214,33 @@ function MOI.delete(model::AbstractModel{T}, vi::VI) where T
     delete!(model.con_to_name, MOI.ConstraintIndex{MOI.SingleVariable, MOI.Semicontinuous{T}}(vi.value))
     delete!(model.con_to_name, MOI.ConstraintIndex{MOI.SingleVariable, MOI.Semiinteger{T}}(vi.value))
 end
-function MOI.delete(model::AbstractModel, vis::Vector{VI})
+function MOI.delete(model::AbstractModel, vi::MOI.VariableIndex)
+    _delete_variable(model, vi)
+    # `VectorOfVariables` constraints with sets not supporting dimension update
+    # were either deleted or an error was thrown. The rest is modified now.
+    broadcastcall(constrs -> _remove_variable(constrs, vi), model)
+    model.objective = remove_variable(model.objective, vi)
+end
+function MOI.delete(model::AbstractModel, vis::Vector{MOI.VariableIndex})
+    if isempty(vis)
+        # In `keep`, we assume that `model.variable_indices !== nothing` so
+        # at least one variable need to be deleted.
+        return
+    end
     # Delete `VectorOfVariables(vis)` constraints as otherwise, it will error
     # when removing variables one by one.
     vov_to_remove = broadcastvcat(constrs -> _vector_of_variables_with(constrs, vis), model)
     for ci in vov_to_remove
         MOI.delete(model, ci)
     end
     for vi in vis
-        MOI.delete(model, vi)
+        _delete_variable(model, vi)
     end
+    # `VectorOfVariables` constraints with sets not supporting dimension update
+    # were either deleted or an error was thrown. The rest is modified now.
+    keep(vi::MOI.VariableIndex) = vi in model.variable_indices
+    model.objective = filter_variables(keep, model.objective)
+    broadcastcall(constrs -> _filter_variables(keep, constrs), model)
 end
 
 function MOI.is_valid(model::AbstractModel,

test/Utilities/model.jl

@@ -200,17 +200,23 @@ end
     @test 1 == @inferred MOI.get(model, MOI.NumberOfConstraints{MOI.VectorAffineFunction{Int},MOI.SecondOrderCone}())
     @test MOI.get(model, MOI.ConstraintFunction(), c6).constants == f6.constants
 
+    fx = MOI.SingleVariable(x)
+    fy = MOI.SingleVariable(y)
+    obj = 1fx + 2fy
+    MOI.set(model, MOI.ObjectiveFunction{typeof(obj)}(), obj)
     message = string("Cannot delete variable as it is constrained with other",
                      " variables in a `MOI.VectorOfVariables`.")
     err = MOI.DeleteNotAllowed(y, message)
     @test_throws err MOI.delete(model, y)
+    @test MOI.get(model, MOI.ObjectiveFunction{typeof(obj)}()) ≈ 1fx + 2fy
 
     @test MOI.is_valid(model, c8)
     MOI.delete(model, c8)
     @test !MOI.is_valid(model, c8)
     @test 0 == @inferred MOI.get(model, MOI.NumberOfConstraints{MOI.VectorOfVariables,MOI.SecondOrderCone}())
 
     MOI.delete(model, y)
+    @test MOI.get(model, MOI.ObjectiveFunction{typeof(obj)}()) ≈ 1fx
 
     f = MOI.get(model, MOI.ConstraintFunction(), c2)
     @test f.affine_terms == MOI.VectorAffineTerm.([1, 2], MOI.ScalarAffineTerm.([3, 1], [x, x]))