Remove symbol in object dictionary with deletion of object

src/JuMP.jl

@@ -438,7 +438,17 @@ function num_nl_constraints(model::Model)
     return model.nlp_data !== nothing ? length(model.nlp_data.nlconstr) : 0
 end
 
-# TODO(IainNZ): Document these too.
+"""
+    object_dictionary(model::Model)
+
+Returns a dictionary mapping symbols to objects. This dictionary is used
+to retrieve the object corresponding to the symbol `symbol::Symbol` in
+`model[symbol]`. Objects are registered to a specific symbol in the macros.
+For instance, `@variable(model, x[1:2, 1:2])` registers the array of variables
+`x` to the symbol `:x`.
+
+A method should be defined for any subtype of `AbstractModel`.
+"""
 object_dictionary(model::Model) = model.obj_dict
 
 """
@@ -943,15 +953,63 @@ function Base.getindex(m::JuMP.AbstractModel, name::Symbol)
 end
 
 """
-    Base.setindex!(m::JuMP.Model, value, name::Symbol)
+    Base.setindex!(model::JuMP.AbstractModel, value, name::Symbol)
+
+stores the object `value` in the model `model` using so that it can be accessed
+via `getindex`.  Can be called with `[]` syntax.
+"""
+function Base.setindex!(model::AbstractModel, value, name::Symbol)
+    object_dictionary(model)[name] = value
+end
+
+"""
+    delete(model::JuMP.AbstractModel, name::Symbol)
+
+Delete the variable(s) or constraint(s) associated with the given name which
+were added to the model. It also removes its association with `name` so that
+new variable(s) or constraint(s) can be associated with this name.
 
-stores the object `value` in the model `m` using so that it can be accessed via `getindex`.  Can be called with `[]` syntax.
+### Example
+
+Using `delete(model, x)`, the variable is deleted from the model but the name
+`:x` is still associated to the variable `x`, even if it is not valid anymore.
+```jldoctest delete_bad; setup = :(using JuMP; model = Model()), filter = r"Stacktrace:.*"s
+julia> @variable(model, x)
+x
+
+julia> delete(model, x)
+
+julia> is_valid(model, x)
+false
+
+julia> @variable(model, x[1:2])
+ERROR: An object of name x is already attached to this model. If this is intended, consider using the anonymous construction syntax, e.g., x = @variable(model, [1:N], ...) where the name of the object does not appear inside the macro.
+Stacktrace:
+ [1] error(::String) at ./error.jl:33
+ [2] _error_if_cannot_register(::Model, ::Symbol) at /home/blegat/.julia/dev/JuMP/src/macros.jl:52
+ [3] top-level scope at /home/blegat/.julia/dev/JuMP/src/macros.jl:75
+```
+To delete the association as well, use `delete(model, :x)` instead.
+```jldoctest delete_good; setup = :(using JuMP; model = Model())
+julia> @variable(model, x)
+x
+
+julia> delete(model, :x)
+
+julia> is_valid(model, x)
+false
+
+julia> @variable(model, x[1:2])
+2-element Array{VariableRef,1}:
+ x[1]
+ x[2]
+```
 """
-function Base.setindex!(m::JuMP.Model, value, name::Symbol)
-    # if haskey(m.obj_dict, name)
-    #     warn("Overwriting the object $name stored in the model. Consider using anonymous variables and constraints instead")
-    # end
-    m.obj_dict[name] = value
+function delete(model::AbstractModel, name::Symbol)
+    value = model[name]
+    delete.(model, value)
+    delete!(object_dictionary(model), name)
+    return
 end
 
 """

src/copy.jl

@@ -93,7 +93,7 @@ function copy_model(model::Model)
     reference_map = ReferenceMap(new_model, index_map)
 
     for (name, value) in object_dictionary(model)
-        new_model.obj_dict[name] = getindex.(reference_map, value)
+        new_model[name] = getindex.(reference_map, value)
     end
 
     for (key, data) in model.ext

src/macros.jl

@@ -90,7 +90,7 @@ function _macro_assign_and_return(code, variable, name;
           end)
         $variable = $code
         $(if model_for_registering !== nothing
-            :(object_dictionary($model_for_registering)[$(quot(name))] =
+            :($model_for_registering[$(quot(name))] =
               $variable)
           end)
         # This assignment should be in the scope calling the macro

test/JuMPExtension.jl

@@ -73,10 +73,12 @@ function JuMP.delete(model::MyModel, vref::MyVariableRef)
     @assert JuMP.is_valid(model, vref)
     delete!(model.variables, vref.idx)
     delete!(model.var_to_name, vref.idx)
+    return
 end
 function JuMP.delete(model::MyModel, vrefs::Vector{MyVariableRef})
     JuMP.delete.(model, vrefs)
 end
+
 function JuMP.is_valid(model::MyModel, vref::MyVariableRef)
     return (model === vref.model &&
             vref.idx in keys(model.variables))

test/constraint.jl

@@ -122,6 +122,29 @@ function test_delete_constraints(ModelType, ::Any)
     @test_throws Exception JuMP.delete(second_model, constraint_ref)
 end
 
+function test_delete_constraints_name(ModelType, ::Any)
+    model = ModelType()
+    @variable(model, x)
+    @constraint(model, con_ref, 2x <= 1)
+    @test JuMP.is_valid(model, con_ref)
+    @test model[:con_ref] === con_ref
+    JuMP.delete(model, :con_ref)
+    @test !JuMP.is_valid(model, con_ref)
+    @test_throws KeyError(:con_ref) model[:con_ref]
+    @constraint(model, con_ref[i=2:3], 3x >= i)
+    for i in 2:3
+        @test JuMP.is_valid(model, con_ref[i])
+        con = constraint_object(con_ref[i])
+        @test isequal_canonical(jump_function(con), 3x)
+        @test moi_set(con) == MOI.GreaterThan(float(i))
+    end
+    @test model[:con_ref] === con_ref
+    JuMP.delete(model, :con_ref)
+    for i in 2:3
+        @test !JuMP.is_valid(model, con_ref[i])
+    end
+end
+
 function test_batch_delete_constraints(ModelType, ::Any)
     model = ModelType()
     @variable(model, x[1:9])

test/variable.jl

@@ -178,6 +178,26 @@ function test_variable_anonymous(ModelType, ::Any)
     @test "" == @inferred JuMP.name(x[2])
 end
 
+function test_variable_delete_name(ModelType, ::Any)
+    model = ModelType()
+    @variable(model, x)
+    @test JuMP.is_valid(model, x)
+    @test model[:x] === x
+    JuMP.delete(model, :x)
+    @test !JuMP.is_valid(model, x)
+    @test_throws KeyError(:x) model[:x]
+    @variable(model, x[i = 1:10; iseven(i)])
+    for i in 2:2:10
+        @test JuMP.is_valid(model, x[i])
+    end
+    @test model[:x] === x
+    JuMP.delete(model, :x)
+    for i in 2:2:10
+        @test !JuMP.is_valid(model, x[i])
+    end
+    @test_throws KeyError(:x) model[:x]
+end
+
 function test_variable_is_valid_delete(ModelType, ::Any)
     model = ModelType()
     @variable(model, x)