Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MOI methods: delete and modify #584

Merged
merged 5 commits into from
Aug 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions src/MOIwrapper.jl
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,84 @@ function MOI.add_constraint(
return constrid
end

############################################################################################
# Delete and modify variable
############################################################################################
function MOI.delete(model::Coluna.Optimizer, vi::MOI.VariableIndex)
MOI.modify(model, MoiObjective(), MOI.ScalarCoefficientChange(vi, 0.0))
for (ci, _) in model.constrs_on_single_var_to_vars
if ci.value == vi.value
MOI.delete(model, ci)
break
end
end
for (ci, _) in model.constrs
MOI.modify(model, ci, MOI.ScalarCoefficientChange(vi, 0.0))
end
varid = getid(model.vars[vi])
delete!(get_original_formulation(model.inner), varid)
delete!(model.moi_varids, varid)
delete!(model.vars, vi)
delete!(model.env.varids, vi)
return
end

function MOI.modify(
model::Coluna.Optimizer, ::MOI.ObjectiveFunction{MOI.ScalarAffineFunction{Float64}},
change::MathOptInterface.ScalarCoefficientChange{Float64}
)
setperencost!(
get_original_formulation(model.inner), model.vars[change.variable], change.new_coefficient
)
return
end

############################################################################################
# Delete and modify constraint
############################################################################################
# issue #583
# function MOI.delete(
# model::Coluna.Optimizer, ci::MOI.ConstraintIndex{F,S}
# ) where {F<:MOI.SingleVariable,S}
# return
# end

function MOI.delete(
model::Coluna.Optimizer, ci::MOI.ConstraintIndex{F,S}
) where {F<:MOI.ScalarAffineFunction{Float64},S}
constrid = getid(model.constrs[ci])
orig_form = get_original_formulation(model.inner)
coefmatrix = getcoefmatrix(orig_form)
varids = VarId[]
for (varid, _) in @view coefmatrix[constrid, :]
push!(varids, varid)
end
Comment on lines +261 to +263
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should change the coefficients in MathProg.delete!.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah indeed

for varid in varids
coefmatrix[constrid, varid] = 0.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you do :

for (varid, _) in @view coefmatrix[constrid, :]
        coefmatrix[constrid, varid] = 0.0
end

Copy link
Contributor

@guimarqu guimarqu Aug 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it enters in an undefined behavior because the size of the array that stores the coefficient matrix may change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, I got an error for accessing an invalid index.

end
delete!(orig_form, constrid)
delete!(model.constrs, ci)
return
end

function MOI.modify(
model::Coluna.Optimizer, ci::MOI.ConstraintIndex{F,S},
change::MOI.ScalarConstantChange{Float64}
) where {F<:MOI.ScalarAffineFunction{Float64},S}
setperenrhs!(get_original_formulation(model.inner), model.constrs[ci], change.new_constant)
return
end

function MOI.modify(
model::Coluna.Optimizer, ci::MOI.ConstraintIndex{F,S},
change::MOI.ScalarCoefficientChange{Float64}
) where {F<:MOI.ScalarAffineFunction{Float64},S}
varid = getid(model.vars[change.variable])
constrid = getid(model.constrs[ci])
getcoefmatrix(get_original_formulation(model.inner))[constrid, varid] = change.new_coefficient
return
end

############################################################################################
# Get variables
############################################################################################
Expand Down
2 changes: 1 addition & 1 deletion src/MathProg/MathProg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import TimerOutputs
import ..Coluna # for NestedEnum (types.jl:210)
using ..ColunaBase

import Base: haskey, length, iterate, diff
import Base: haskey, length, iterate, diff, delete!

using DynamicSparseArrays, Logging, Printf

Expand Down
20 changes: 20 additions & 0 deletions src/MathProg/varconstr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,26 @@ function deactivate!(form::Formulation, f::Function)
return
end

## delete
"""
delete!(formulation, varconstrid)
delete!(formulation, varconstr)

Delete a variable or a constraint from a formulation.
"""
function Base.delete!(form::Formulation, varid::VarId)
delete!(form.manager.vars, varid)
delete!(form.buffer.var_buffer.added, varid)
return
end
Base.delete!(form::Formulation, var::Variable) = delete!(form, getid(var))

function Base.delete!(form::Formulation, constrid::ConstrId)
delete!(form.buffer.constr_buffer.added, constrid)
delete!(form.manager.constrs, constrid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
delete!(form.manager.constrs, constrid)
delete!(form.manager.constrs, constrid)
return

end
Base.delete!(form::Formulation, constr::Constraint) = delete!(form, getid(constr))

## explicit
"""
isexplicit(formulation, varconstr)
Expand Down
16 changes: 6 additions & 10 deletions test/MathOptInterface/MOI_wrapper.jl
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,7 @@ end

const UNSUPPORTED_TESTS = [
"solve_qcp_edge_cases", # Quadratic constraints not supported
"delete_nonnegative_variables", # variable deletion not supported
"delete_variable", # variable deletion not supported
"delete_variables", # variable deletion not supported
"delete_nonnegative_variables", # `VectorOfVariables`-in-`Nonnegatives` not supported
"variablenames", # Coluna retrieves the name of the variable
"delete_soc_variables", # soc variables not supported
"solve_qp_edge_cases", # Quadratic objective not supported
Expand Down Expand Up @@ -192,12 +190,10 @@ const CONSTRAINTDUAL_SINGLEVAR = [
"linear14"
]

const MODIFY_DELETE = [
# BUG
"linear1", # modify
"linear5", # modify
"linear11", # delete
"linear14" # delete
const DELETE_SINGLEVAR_CONSTR = [
# BUG: issue #583
"linear5",
"linear14"
]

const UNCOVERED_TERMINATION_STATUS = [
Expand Down Expand Up @@ -240,7 +236,7 @@ end

@testset "Continuous Linear" begin
MOIT.contlineartest(BRIDGED, CONFIG, vcat(
CONSTRAINTDUAL_SINGLEVAR, MODIFY_DELETE, UNCOVERED_TERMINATION_STATUS, SET_CONSTRAINTSET, [
CONSTRAINTDUAL_SINGLEVAR, DELETE_SINGLEVAR_CONSTR, UNCOVERED_TERMINATION_STATUS, SET_CONSTRAINTSET, [
"partial_start" # VariablePrimalStart not supported
]
))
Expand Down