Merge pull request #774 from mimiframework/delete

add `delete_unbound_comp_params` kwarg to `delete!`

Author: corakingdon

docs/src/ref/ref_API.md

@@ -7,6 +7,7 @@ Model
 add_comp!  
 connect_param!
 create_marginal_model
+delete_param!
 dim_count
 dim_keys
 dim_key_dict

src/Mimi.jl

@@ -19,6 +19,7 @@ export
     # components,
     connect_param!,
     create_marginal_model,
+    delete_param!,
     dim_count,
     dim_keys,
     dim_key_dict,

src/core/defs.jl

@@ -58,28 +58,65 @@ find_first_period(comp_def::AbstractComponentDef) = @or(first_period(comp_def),
 find_last_period(comp_def::AbstractComponentDef) = @or(last_period(comp_def), last_period(get_root(comp_def)))
 
 """
-    delete!(obj::AbstractCompositeComponentDef, component::Symbol)
+    delete!(md::ModelDef, comp_name::Symbol; deep::Bool=false)
 
-Delete a `component` by name from composite `ccd`.
+Delete a `component` by name from `md`.
+If `deep=true` then any external model parameters connected only to 
+this component will also be deleted.
 """
-function Base.delete!(ccd::AbstractCompositeComponentDef, comp_name::Symbol)
-    if ! has_comp(ccd, comp_name)
+function Base.delete!(md::ModelDef, comp_name::Symbol; deep::Bool=false)
+    if ! has_comp(md, comp_name)
         error("Cannot delete '$comp_name': component does not exist.")
     end
 
-    comp_def = compdef(ccd, comp_name)
-    delete!(ccd.namespace, comp_name)
+    comp_def = compdef(md, comp_name)
+    delete!(md.namespace, comp_name)
 
     # Remove references to the deleted comp
     comp_path = comp_def.comp_path
 
-    # TBD: find and delete external_params associated with deleted component? Currently no record of this.
-
+    # Remove internal parameter connections 
     ipc_filter = x -> x.src_comp_path != comp_path && x.dst_comp_path != comp_path
-    filter!(ipc_filter, ccd.internal_param_conns)
+    filter!(ipc_filter, md.internal_param_conns)
+
+    # Remove external parameter connections
+
+    if deep # Find and delete external_params that were connected only to the deleted component if specified
+        # Get all external parameters this component is connected to
+        comp_ext_params = map(x -> x.external_param, filter(x -> x.comp_path == comp_path, md.external_param_conns))
+
+        # Identify which ones are not connected to any other components
+        unbound_filter = x -> length(filter(epc -> epc.external_param == x, md.external_param_conns)) == 1
+        unbound_comp_params = filter(unbound_filter, comp_ext_params)
+
+        # Delete these parameters (the delete_param! function also deletes the associated external_param_conns)
+        [delete_param!(md, param_name) for param_name in unbound_comp_params]
+
+    else # only delete the external connections for this component but leave all external parameters
+        epc_filter = x -> x.comp_path != comp_path
+        filter!(epc_filter, md.external_param_conns)
+    end
+    dirty!(md)
+end
+
+"""
+    delete_param!(md::ModelDef, external_param_name::Symbol)
+
+Delete `external_param_name` from `md`'s list of external parameters, and also 
+remove all external parameters connections that were connected to `external_param_name`.
+"""
+function delete_param!(md::ModelDef, external_param_name::Symbol)
+    if external_param_name in keys(md.external_params)
+        delete!(md.external_params, external_param_name)
+    else
+        error("Cannot delete $external_param_name, not found in external parameter list.")
+    end
+    
+    # Remove external parameter connections
+    epc_filter = x -> x.external_param != external_param_name
+    filter!(epc_filter, md.external_param_conns)
 
-    epc_filter = x -> x.comp_path != comp_path
-    filter!(epc_filter, ccd.external_param_conns)
+    dirty!(md)
 end
 
 @delegate Base.haskey(comp::AbstractComponentDef, key::Symbol) => namespace

src/core/model.jl

@@ -369,11 +369,21 @@ Add a scalar type parameter `name` with value `value` to the model `m`.
 @delegate set_external_scalar_param!(m::Model, name::Symbol, value::Any) => md
 
 """
-    delete!(m::Model, component::Symbol
+    delete!(m::Model, component::Symbol; deep::Bool=false)
 
-Delete a `component`` by name from a model `m`'s ModelDef, and nullify the ModelInstance.
+Delete a `component` by name from a model `m`'s ModelDef, and nullify the ModelInstance.
+If `deep=true` then any external model parameters connected only to 
+this component will also be deleted.
 """
-@delegate Base.delete!(m::Model, comp_name::Symbol) => md
+@delegate Base.delete!(m::Model, comp_name::Symbol; deep::Bool=false) => md
+
+"""
+    delete_param!(m::Model, external_param_name::Symbol)
+
+Delete `external_param_name` from a model `m`'s ModelDef's list of external parameters, and
+also remove all external parameters connections that were connected to `external_param_name`.
+"""
+@delegate delete_param!(m::Model, external_param_name::Symbol) => md
 
 """
     set_param!(m::Model, comp_name::Symbol, param_name::Symbol, value; dims=nothing)

test/runtests.jl

@@ -47,6 +47,9 @@ Electron.prep_test_env()
     @info("test_model_structure_variabletimestep.jl")
     @time include("test_model_structure_variabletimestep.jl")
 
+    @info("test_delete.jl")
+    @time include("test_delete.jl")
+
     @info("test_replace_comp.jl")
     @time include("test_replace_comp.jl")
 

test/test_delete.jl

@@ -0,0 +1,51 @@
+module TestDelete
+
+# Test the behavior of the `delete!` function with and without the `deep` kwarg.
+
+using Mimi
+using Test
+
+@defcomp A begin
+    p1 = Parameter()
+    p2 = Parameter()
+end
+
+function _get_model()
+    m = Model()
+    set_dimension!(m, :time, 1:2)
+    add_comp!(m, A, :A1)
+    add_comp!(m, A, :A2)
+    set_param!(m, :p1, 1)
+    set_param!(m, :A1, :p2, :p2_A1, 21)
+    set_param!(m, :A2, :p2, :p2_A2, 22)
+    return m
+end
+
+# Test component deletion without removing unbound component parameters
+m1 = _get_model()
+run(m1)
+@test length(Mimi.components(m1)) == 2
+@test length(m1.md.external_param_conns) == 4   # two components with two connections each
+@test length(m1.md.external_params) == 3        # three total external params 
+delete!(m1, :A1)    
+run(m1) # run before and after to test that `delete!` properly "dirties" the model, and builds a new instance on the next run
+@test length(Mimi.components(m1)) == 1
+@test length(m1.md.external_param_conns) == 2   # Component A1 deleted, so only two connections left
+@test length(m1.md.external_params) == 3        # but all three external params remain
+@test :p2_A1 in keys(m1.md.external_params)
+
+# Test component deletion that removes unbound component parameters
+m2 = _get_model()
+delete!(m2, :A1, deep = true)
+@test length(Mimi.components(m2.md)) == 1
+@test length(m2.md.external_params) == 2        # :p2_A1 has been removed
+@test !(:p2_A1 in keys(m2.md.external_params))
+
+# Test the `delete_param! function on its own
+m3 = _get_model()
+run(m3)
+delete_param!(m3, :p1)
+@test_throws ErrorException run(m3)     # will not be able to run because p1 in both components aren't connected to anything
+@test length(m3.md.external_params) == 2
+@test length(m3.md.external_param_conns) == 2   # The external param connections to p1 have also been removed
+end