From 285ffa99b8cf633d98b6b9e591d39a618171a199 Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Fri, 23 Oct 2020 11:10:48 -0400 Subject: [PATCH 1/7] add `delete_unbound_comp_params` kwarg to `delete!`; add `delete_param!` function --- docs/src/ref/ref_API.md | 1 + src/Mimi.jl | 1 + src/core/defs.jl | 59 +++++++++++++++++++++++++++++++++-------- src/core/model.jl | 8 +++--- test/runtests.jl | 3 +++ test/test_delete.jl | 41 ++++++++++++++++++++++++++++ 6 files changed, 99 insertions(+), 14 deletions(-) create mode 100644 test/test_delete.jl diff --git a/docs/src/ref/ref_API.md b/docs/src/ref/ref_API.md index aa06eb240..cda519728 100644 --- a/docs/src/ref/ref_API.md +++ b/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 diff --git a/src/Mimi.jl b/src/Mimi.jl index e934e1d3e..45770058f 100644 --- a/src/Mimi.jl +++ b/src/Mimi.jl @@ -19,6 +19,7 @@ export # components, connect_param!, create_marginal_model, + delete_param!, dim_count, dim_keys, dim_key_dict, diff --git a/src/core/defs.jl b/src/core/defs.jl index 1fdc9926e..72eec38ef 100644 --- a/src/core/defs.jl +++ b/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; delete_unbound_comp_params::Bool=false) -Delete a `component` by name from composite `ccd`. +Delete a `component` by name from `md`. +If `delete_unbound_comp_params=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; delete_unbound_comp_params::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 delete_unbound_comp_params # 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)) + println(comp_ext_params) - epc_filter = x -> x.comp_path != comp_path - filter!(epc_filter, ccd.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) + println(unbound_comp_params) + + # Delete these parameters (the delete_param! function also deletes the associated external_param_conns) + [delete_param!(md, param_name, delete_connections=true) 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 +end + +""" + delete_param!(md::ModelDef, external_param_name::Symbol; delete_connections::Bool=true) + +Delete `external_param_name` from `md`'s list of external parameters. If `delete_connections=true`, +also remove all external parameters connections that were connected to `external_param_name`. +""" +function delete_param!(md::ModelDef, external_param_name::Symbol; delete_connections::Bool=true) + 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 + + if delete_connections + epc_filter = x -> x.external_param == external_param_name + filter!(epc_filter, md.external_param_conns) + end end @delegate Base.haskey(comp::AbstractComponentDef, key::Symbol) => namespace diff --git a/src/core/model.jl b/src/core/model.jl index 11c5f48b7..d8cb163c1 100644 --- a/src/core/model.jl +++ b/src/core/model.jl @@ -369,11 +369,13 @@ 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; delete_unbound_comp_params::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 `delete_unbound_comp_params=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; delete_unbound_comp_params::Bool=false) => md """ set_param!(m::Model, comp_name::Symbol, param_name::Symbol, value; dims=nothing) diff --git a/test/runtests.jl b/test/runtests.jl index 5c8220abc..ac2c4e1d4 100644 --- a/test/runtests.jl +++ b/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") diff --git a/test/test_delete.jl b/test/test_delete.jl new file mode 100644 index 000000000..32b41eb0a --- /dev/null +++ b/test/test_delete.jl @@ -0,0 +1,41 @@ +module TestDelete + +# Test the behavior of the `delete!` function with and without the `delete_unbound_comp_params` kwarg. + +using Mimi +using Test + +@defcomp A begin + p1 = Parameter() + p2 = Parameter() +end + +function _get_model() + m = Model() + 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() +@test length(Mimi.components(m1.md)) == 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) +@test length(Mimi.components(m1.md)) == 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(m2.md.external_params) + +# Test component deletion that removes unbound component parameters +m2 = _get_model() +delete!(m2, :A1, delete_unbound_comp_params = 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)) + +end \ No newline at end of file From 6d631a6ebc4fbd2c629990abe4d7c9ca09ea2074 Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Fri, 23 Oct 2020 11:12:52 -0400 Subject: [PATCH 2/7] remove print statements --- src/core/defs.jl | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/core/defs.jl b/src/core/defs.jl index 72eec38ef..52232b5f7 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -84,12 +84,10 @@ function Base.delete!(md::ModelDef, comp_name::Symbol; delete_unbound_comp_param if delete_unbound_comp_params # 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)) - println(comp_ext_params) # 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) - println(unbound_comp_params) # Delete these parameters (the delete_param! function also deletes the associated external_param_conns) [delete_param!(md, param_name, delete_connections=true) for param_name in unbound_comp_params] From 4ea7a456d378839233c851f2b7a4f9c625a71774 Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Fri, 23 Oct 2020 11:28:14 -0400 Subject: [PATCH 3/7] add test for delete_param! --- src/core/defs.jl | 2 +- src/core/model.jl | 8 ++++++++ test/test_delete.jl | 10 +++++++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/core/defs.jl b/src/core/defs.jl index 52232b5f7..2346c3743 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -112,7 +112,7 @@ function delete_param!(md::ModelDef, external_param_name::Symbol; delete_connect end if delete_connections - epc_filter = x -> x.external_param == external_param_name + epc_filter = x -> x.external_param != external_param_name filter!(epc_filter, md.external_param_conns) end end diff --git a/src/core/model.jl b/src/core/model.jl index d8cb163c1..0f69e9c1c 100644 --- a/src/core/model.jl +++ b/src/core/model.jl @@ -377,6 +377,14 @@ this component will also be deleted. """ @delegate Base.delete!(m::Model, comp_name::Symbol; delete_unbound_comp_params::Bool=false) => md +""" + delete_param!(m::Model, external_param_name::Symbol; delete_connections::Bool=true) + +Delete `external_param_name` from a model `m`'s ModelDef's list of external parameters. If `delete_connections=true`, +also remove all external parameters connections that were connected to `external_param_name`. +""" +@delegate delete_param!(m::Model, external_param_name::Symbol; delete_connections::Bool=true) => md + """ set_param!(m::Model, comp_name::Symbol, param_name::Symbol, value; dims=nothing) diff --git a/test/test_delete.jl b/test/test_delete.jl index 32b41eb0a..26a9c8925 100644 --- a/test/test_delete.jl +++ b/test/test_delete.jl @@ -29,7 +29,7 @@ delete!(m1, :A1) @test length(Mimi.components(m1.md)) == 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(m2.md.external_params) +@test :p2_A1 in keys(m1.md.external_params) # Test component deletion that removes unbound component parameters m2 = _get_model() @@ -38,4 +38,12 @@ delete!(m2, :A1, delete_unbound_comp_params = true) @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() +delete_param!(m3, :p1, delete_connections = false) +@test length(m3.md.external_params) == 2 +@test length(m3.md.external_param_conns) == 4 # All connections still remain +delete_param!(m3, :p2_A1, delete_connections = true) +@test length(m3.md.external_params) == 1 +@test length(m3.md.external_param_conns) == 3 # All connections still remain end \ No newline at end of file From 09053370712aaac2a630e154d8ff249a1592e8d4 Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Fri, 23 Oct 2020 13:16:53 -0400 Subject: [PATCH 4/7] fix comment --- test/test_delete.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_delete.jl b/test/test_delete.jl index 26a9c8925..7a5972991 100644 --- a/test/test_delete.jl +++ b/test/test_delete.jl @@ -45,5 +45,5 @@ delete_param!(m3, :p1, delete_connections = false) @test length(m3.md.external_param_conns) == 4 # All connections still remain delete_param!(m3, :p2_A1, delete_connections = true) @test length(m3.md.external_params) == 1 -@test length(m3.md.external_param_conns) == 3 # All connections still remain +@test length(m3.md.external_param_conns) == 3 # p2_A1 connection was deleted end \ No newline at end of file From e127cfa9ad6167a1112513e4fefda4ffcb1c673f Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Fri, 23 Oct 2020 13:40:02 -0400 Subject: [PATCH 5/7] make `delete!` and `delete_param!` properly "dirty" the model --- src/core/defs.jl | 2 ++ test/test_delete.jl | 11 ++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/core/defs.jl b/src/core/defs.jl index 2346c3743..180362305 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -96,6 +96,7 @@ function Base.delete!(md::ModelDef, comp_name::Symbol; delete_unbound_comp_param epc_filter = x -> x.comp_path != comp_path filter!(epc_filter, md.external_param_conns) end + dirty!(md) end """ @@ -115,6 +116,7 @@ function delete_param!(md::ModelDef, external_param_name::Symbol; delete_connect epc_filter = x -> x.external_param != external_param_name filter!(epc_filter, md.external_param_conns) end + dirty!(md) end @delegate Base.haskey(comp::AbstractComponentDef, key::Symbol) => namespace diff --git a/test/test_delete.jl b/test/test_delete.jl index 7a5972991..d2ffdaf50 100644 --- a/test/test_delete.jl +++ b/test/test_delete.jl @@ -12,6 +12,7 @@ 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) @@ -22,11 +23,13 @@ end # Test component deletion without removing unbound component parameters m1 = _get_model() -@test length(Mimi.components(m1.md)) == 2 +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) -@test length(Mimi.components(m1.md)) == 1 +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) @@ -40,7 +43,9 @@ delete!(m2, :A1, delete_unbound_comp_params = true) # Test the `delete_param! function on its own m3 = _get_model() +run(m3) delete_param!(m3, :p1, delete_connections = false) +@test_throws ErrorException run(m3) # will error because there are connections that reference p1 @test length(m3.md.external_params) == 2 @test length(m3.md.external_param_conns) == 4 # All connections still remain delete_param!(m3, :p2_A1, delete_connections = true) From 3ed6b2c8721a383af824ca058c8eeb8a7ef49c0b Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Fri, 23 Oct 2020 16:59:00 -0400 Subject: [PATCH 6/7] remove kwarg option in `delete_param!` --- src/core/defs.jl | 18 +++++++++--------- src/core/model.jl | 6 +++--- test/test_delete.jl | 9 +++------ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/core/defs.jl b/src/core/defs.jl index 180362305..b5de01392 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -90,7 +90,7 @@ function Base.delete!(md::ModelDef, comp_name::Symbol; delete_unbound_comp_param 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, delete_connections=true) for param_name in unbound_comp_params] + [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 @@ -100,22 +100,22 @@ function Base.delete!(md::ModelDef, comp_name::Symbol; delete_unbound_comp_param end """ - delete_param!(md::ModelDef, external_param_name::Symbol; delete_connections::Bool=true) + delete_param!(md::ModelDef, external_param_name::Symbol) -Delete `external_param_name` from `md`'s list of external parameters. If `delete_connections=true`, -also remove all external parameters connections that were connected to `external_param_name`. +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; delete_connections::Bool=true) +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 - if delete_connections - epc_filter = x -> x.external_param != external_param_name - filter!(epc_filter, md.external_param_conns) - end + # Remove external parameter connections + epc_filter = x -> x.external_param != external_param_name + filter!(epc_filter, md.external_param_conns) + dirty!(md) end diff --git a/src/core/model.jl b/src/core/model.jl index 0f69e9c1c..51b00f286 100644 --- a/src/core/model.jl +++ b/src/core/model.jl @@ -378,12 +378,12 @@ this component will also be deleted. @delegate Base.delete!(m::Model, comp_name::Symbol; delete_unbound_comp_params::Bool=false) => md """ - delete_param!(m::Model, external_param_name::Symbol; delete_connections::Bool=true) + delete_param!(m::Model, external_param_name::Symbol) -Delete `external_param_name` from a model `m`'s ModelDef's list of external parameters. If `delete_connections=true`, +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; delete_connections::Bool=true) => md +@delegate delete_param!(m::Model, external_param_name::Symbol) => md """ set_param!(m::Model, comp_name::Symbol, param_name::Symbol, value; dims=nothing) diff --git a/test/test_delete.jl b/test/test_delete.jl index d2ffdaf50..347ae9e53 100644 --- a/test/test_delete.jl +++ b/test/test_delete.jl @@ -44,11 +44,8 @@ delete!(m2, :A1, delete_unbound_comp_params = true) # Test the `delete_param! function on its own m3 = _get_model() run(m3) -delete_param!(m3, :p1, delete_connections = false) -@test_throws ErrorException run(m3) # will error because there are connections that reference p1 +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) == 4 # All connections still remain -delete_param!(m3, :p2_A1, delete_connections = true) -@test length(m3.md.external_params) == 1 -@test length(m3.md.external_param_conns) == 3 # p2_A1 connection was deleted +@test length(m3.md.external_param_conns) == 2 # The external param connections to p1 have also been removed end \ No newline at end of file From b55d09fc72cd5c366ef7cb357dac1408a3d65d75 Mon Sep 17 00:00:00 2001 From: lrennels Date: Tue, 27 Oct 2020 12:21:30 -0700 Subject: [PATCH 7/7] change keyword arg --- src/core/defs.jl | 8 ++++---- src/core/model.jl | 6 +++--- test/test_delete.jl | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/defs.jl b/src/core/defs.jl index b5de01392..ee21277c9 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -58,13 +58,13 @@ 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!(md::ModelDef, comp_name::Symbol; delete_unbound_comp_params::Bool=false) + delete!(md::ModelDef, comp_name::Symbol; deep::Bool=false) Delete a `component` by name from `md`. -If `delete_unbound_comp_params=true` then any external model parameters connected only to +If `deep=true` then any external model parameters connected only to this component will also be deleted. """ -function Base.delete!(md::ModelDef, comp_name::Symbol; delete_unbound_comp_params::Bool=false) +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 @@ -81,7 +81,7 @@ function Base.delete!(md::ModelDef, comp_name::Symbol; delete_unbound_comp_param # Remove external parameter connections - if delete_unbound_comp_params # Find and delete external_params that were connected only to the deleted component if specified + 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)) diff --git a/src/core/model.jl b/src/core/model.jl index 51b00f286..c0b9493e0 100644 --- a/src/core/model.jl +++ b/src/core/model.jl @@ -369,13 +369,13 @@ 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_unbound_comp_params::Bool=false) + delete!(m::Model, component::Symbol; deep::Bool=false) Delete a `component` by name from a model `m`'s ModelDef, and nullify the ModelInstance. -If `delete_unbound_comp_params=true` then any external model parameters connected only to +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; delete_unbound_comp_params::Bool=false) => md +@delegate Base.delete!(m::Model, comp_name::Symbol; deep::Bool=false) => md """ delete_param!(m::Model, external_param_name::Symbol) diff --git a/test/test_delete.jl b/test/test_delete.jl index 347ae9e53..577233280 100644 --- a/test/test_delete.jl +++ b/test/test_delete.jl @@ -1,6 +1,6 @@ module TestDelete -# Test the behavior of the `delete!` function with and without the `delete_unbound_comp_params` kwarg. +# Test the behavior of the `delete!` function with and without the `deep` kwarg. using Mimi using Test @@ -36,7 +36,7 @@ run(m1) # run before and after to test that `delete!` properly "dirties" the mod # Test component deletion that removes unbound component parameters m2 = _get_model() -delete!(m2, :A1, delete_unbound_comp_params = true) +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))