Skip to content

Fix deletion of constrained variables for CachingOptimizer #905

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 30, 2019
Merged

Fix deletion of constrained variables for CachingOptimizer #905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e56a1e
Fix deletion of constrained variables for CachingOptimizer
blegat Sep 27, 2019
0060461
Split test in two
blegat Sep 28, 2019
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
80 changes: 80 additions & 0 deletions src/Test/UnitTests/variables.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,86 @@ function delete_variables(model::MOI.ModelLike, config::TestConfig)
end
unittests["delete_variables"] = delete_variable

"""
delete_nonnegative_variables(model::MOI.ModelLike, config::TestConfig)

Test adding, and then deleting, nonnegative variables.
"""
function delete_nonnegative_variables(model::MOI.ModelLike, config::TestConfig)
MOI.empty!(model)
@test MOI.is_empty(model)
@test MOI.get(model, MOI.NumberOfVariables()) == 0
v, cv = MOI.add_constrained_variables(model, MOI.Nonnegatives(2))
@test MOI.get(model, MOI.NumberOfVariables()) == 2
MOI.delete(model, v)
@test MOI.get(model, MOI.NumberOfVariables()) == 0
@test !MOI.is_valid(model, v[1])
@test_throws MOI.InvalidIndex(v[1]) MOI.delete(model, v[1])
@test !MOI.is_valid(model, v[2])
@test_throws MOI.InvalidIndex(v[2]) MOI.delete(model, v[2])
@test !MOI.is_valid(model, cv)
v, cv = MOI.add_constrained_variables(model, MOI.Nonnegatives(1))
@test MOI.get(model, MOI.NumberOfVariables()) == 1
MOI.delete(model, v[1])
@test !MOI.is_valid(model, v[1])
@test_throws MOI.InvalidIndex(v[1]) MOI.delete(model, v[1])
@test !MOI.is_valid(model, cv)
@test MOI.get(model, MOI.NumberOfVariables()) == 0
end
unittests["delete_nonnegative_variables"] = delete_nonnegative_variables

"""
update_dimension_nonnegative_variables(model::MOI.ModelLike, config::TestConfig)

Test adding, and then deleting one by one, nonnegative variables.
"""
function update_dimension_nonnegative_variables(model::MOI.ModelLike, config::TestConfig)
MOI.empty!(model)
@test MOI.is_empty(model)
@test MOI.get(model, MOI.NumberOfVariables()) == 0
v, cv = MOI.add_constrained_variables(model, MOI.Nonnegatives(2))
@test MOI.get(model, MOI.NumberOfVariables()) == 2
MOI.delete(model, v[1])
@test !MOI.is_valid(model, v[1])
@test_throws MOI.InvalidIndex(v[1]) MOI.delete(model, v[1])
@test MOI.is_valid(model, cv)
@test MOI.is_valid(model, v[2])
@test MOI.get(model, MOI.ConstraintFunction(), cv) == MOI.VectorOfVariables([v[2]])
@test MOI.get(model, MOI.ConstraintSet(), cv) == MOI.Nonnegatives(1)
MOI.delete(model, v[2])
@test MOI.get(model, MOI.NumberOfVariables()) == 0
@test !MOI.is_valid(model, v[1])
@test_throws MOI.InvalidIndex(v[1]) MOI.delete(model, v[1])
@test !MOI.is_valid(model, v[2])
@test_throws MOI.InvalidIndex(v[2]) MOI.delete(model, v[2])
@test !MOI.is_valid(model, cv)
end
unittests["update_dimension_nonnegative_variables"] = update_dimension_nonnegative_variables

"""
delete_soc_variables(model::MOI.ModelLike, config::TestConfig)

Test adding, and then deleting, second-order cone variables.
"""
function delete_soc_variables(model::MOI.ModelLike, config::TestConfig)
MOI.empty!(model)
@test MOI.is_empty(model)
@test MOI.get(model, MOI.NumberOfVariables()) == 0
v, cv = MOI.add_constrained_variables(model, MOI.SecondOrderCone(3))
@test MOI.get(model, MOI.NumberOfVariables()) == 3
MOI.delete(model, v)
@test MOI.get(model, MOI.NumberOfVariables()) == 0
@test !MOI.is_valid(model, v[1])
@test_throws MOI.InvalidIndex(v[1]) MOI.delete(model, v[1])
@test !MOI.is_valid(model, v[2])
@test_throws MOI.InvalidIndex(v[2]) MOI.delete(model, v[2])
@test !MOI.is_valid(model, cv)
v, cv = MOI.add_constrained_variables(model, MOI.SecondOrderCone(3))
@test MOI.get(model, MOI.NumberOfVariables()) == 3
@test_throws MOI.DeleteNotAllowed MOI.delete(model, v[1])
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm.... I hadn't thought of this case. I guess it's reasonable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Passing existing tests with bridged variables put a lot of stress on the deletion API so as part of adding the variable bridges, we clarified what should be done in these corner cases. The relevant PR is #793

end
unittests["delete_soc_variables"] = delete_soc_variables

"""
getvariable(model::MOI.ModelLike, config::TestConfig)

Expand Down
33 changes: 33 additions & 0 deletions src/Utilities/cachingoptimizer.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,39 @@ function MOI.delete(m::CachingOptimizer, index::MOI.Index)
MOI.delete(m.model_cache, index)
end

function MOI.delete(m::CachingOptimizer, indices::Vector{<:MOI.Index})
if m.state == ATTACHED_OPTIMIZER
for index in indices
if !MOI.is_valid(m, index)
# The index thrown by m.model_cache would be xored
throw(MOI.InvalidIndex(index))
end
end
indices_optimizer = [m.model_to_optimizer_map[index] for index in indices]
if m.mode == AUTOMATIC
try
MOI.delete(m.optimizer, indices_optimizer)
catch err
if err isa MOI.NotAllowedError
reset_optimizer(m)
else
rethrow(err)
end
end
else
MOI.delete(m.optimizer, indices_optimizer)
end
end
# The state may have changed in AUTOMATIC mode since reset_optimizer is
# called in case the deletion is not supported
if m.state == ATTACHED_OPTIMIZER
for index in indices
delete!(m.optimizer_to_model_map, m.model_to_optimizer_map[index])
delete!(m.model_to_optimizer_map, index)
end
end
MOI.delete(m.model_cache, indices)
end

# TODO: add_constraints, transform

Expand Down