From 593e4f131e1074587690be780e387f18ea3c367f Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Tue, 15 Oct 2019 11:49:07 -0400 Subject: [PATCH 1/2] Remove first and last kewyord arguments --- src/core/defs.jl | 123 ++++++++---------- src/core/instances.jl | 8 +- src/core/model.jl | 32 +++-- test/test_components.jl | 5 +- test/test_datum_storage.jl | 15 +-- test/test_dimensions.jl | 5 +- test/test_getdataframe.jl | 10 +- test/test_model_structure_variabletimestep.jl | 18 +-- test/test_parametertypes.jl | 6 +- 9 files changed, 84 insertions(+), 138 deletions(-) diff --git a/src/core/defs.jl b/src/core/defs.jl index d90319d67..f975807ea 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -754,55 +754,39 @@ function import_params!(comp::AbstractComponentDef) end """ - add_comp!(obj::AbstractCompositeComponentDef, comp_def::AbstractComponentDef, - comp_name::Symbol=comp_def.comp_id.comp_name; rename=nothing, - first=nothing, last=nothing, before=nothing, after=nothing) + add_comp!( + obj::AbstractCompositeComponentDef, + comp_def::AbstractComponentDef, + comp_name::Symbol=comp_def.comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + rename::NothingPairList=nothing + ) Add the component `comp_def` to the composite component indicated by `obj`. The component is added at the end of the list unless one of the keywords `before` or `after` is specified. Note that a copy of `comp_id` is made in the composite and assigned the give name. The optional argument `rename` can be a list of pairs indicating `original_name => imported_name`. - -Note: `first` and `last` keywords are currently disabled. """ -function add_comp!(obj::AbstractCompositeComponentDef, comp_def::AbstractComponentDef, - comp_name::Symbol=comp_def.comp_id.comp_name; - first::NothingInt=nothing, last::NothingInt=nothing, - before::NothingSymbol=nothing, after::NothingSymbol=nothing, - rename::NothingPairList=nothing) - - if first !== nothing || last !== nothing - @warn "add_comp!: Keyword arguments 'first' and 'last' are currently disabled." - first = last = nothing - end - - # When adding composites to another composite, we disallow setting first and last periods. - if is_composite(comp_def) && (first !== nothing || last !== nothing) - error("Cannot set first or last period when adding a composite component: $(comp_def.comp_id)") - end +function add_comp!( + obj::AbstractCompositeComponentDef, + comp_def::AbstractComponentDef, + comp_name::Symbol=comp_def.comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + rename::NothingPairList=nothing +) # Check if component being added already exists has_comp(obj, comp_name) && error("Cannot add two components of the same name ($comp_name)") + if before !== nothing && after !== nothing + error("Cannot specify both 'before' and 'after' parameters") + end + # check time constraints if the time dimension has been set if has_dim(obj, :time) # error("Cannot add component to composite without first setting time dimension.") - - # check that first and last are within the model's time index range - time_index = time_labels(obj) - - if first !== nothing && first < time_index[1] - error("Cannot add component $comp_name with first time before first of model's time index range.") - end - - if last !== nothing && last > time_index[end] - error("Cannot add component $comp_name with last time after end of model's time index range.") - end - - if before !== nothing && after !== nothing - error("Cannot specify both 'before' and 'after' parameters") - end - propagate_time!(comp_def, dimension(obj, :time)) end @@ -811,7 +795,6 @@ function add_comp!(obj::AbstractCompositeComponentDef, comp_def::AbstractCompone comp_def.name = comp_name parent!(comp_def, obj) - _set_run_period!(comp_def, first, last) _add_anonymous_dims!(obj, comp_def) _insert_comp!(obj, comp_def, before=before, after=after) @@ -830,31 +813,41 @@ function add_comp!(obj::AbstractCompositeComponentDef, comp_def::AbstractCompone end """ - add_comp!(obj::CompositeComponentDef, comp_id::ComponentId; comp_name::Symbol=comp_id.comp_name, - first=nothing, last=nothing, before=nothing, after=nothing, rename=nothing) + add_comp!( + obj::AbstractCompositeComponentDef, + comp_id::ComponentId, + comp_name::Symbol=comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + rename::NothingPairList=nothing + ) Add the component indicated by `comp_id` to the composite component indicated by `obj`. The component is added at the end of the list unless one of the keywords `before` or `after` is specified. Note that a copy of `comp_id` is made in the composite and assigned the give name. The optional argument `rename` can be a list of pairs indicating `original_name => imported_name`. - -Note: `first` and `last` keywords are currently disabled. """ -function add_comp!(obj::AbstractCompositeComponentDef, comp_id::ComponentId, - comp_name::Symbol=comp_id.comp_name; - first::NothingInt=nothing, last::NothingInt=nothing, - before::NothingSymbol=nothing, after::NothingSymbol=nothing, - rename::NothingPairList=nothing) +function add_comp!( + obj::AbstractCompositeComponentDef, + comp_id::ComponentId, + comp_name::Symbol=comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + rename::NothingPairList=nothing +) # println("Adding component $comp_id as :$comp_name") - add_comp!(obj, compdef(comp_id), comp_name, - first=first, last=last, before=before, after=after, rename=rename) + add_comp!(obj, compdef(comp_id), comp_name, before=before, after=after, rename=rename) end """ - replace_comp!(obj::CompositeComponentDef, comp_id::ComponentId, comp_name::Symbol=comp_id.comp_name; - first::NothingInt=nothing, last::NothingInt=nothing, - before::NothingSymbol=nothing, after::NothingSymbol=nothing, - reconnect::Bool=true) + replace_comp!( + obj::AbstractCompositeComponentDef, + comp_id::ComponentId, + comp_name::Symbol=comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + reconnect::Bool=true + ) Replace the component with name `comp_name` in composite component definition `obj` with the component `comp_id` using the same name. The component is added in the same position as the @@ -862,19 +855,15 @@ old component, unless one of the keywords `before` or `after` is specified. The added with the same first and last values, unless the keywords `first` or `last` are specified. Optional boolean argument `reconnect` with default value `true` indicates whether the existing parameter connections should be maintained in the new component. Returns the added comp def. - -Note: `first` and `last` keywords are currently disabled. """ -function replace_comp!(obj::AbstractCompositeComponentDef, comp_id::ComponentId, - comp_name::Symbol=comp_id.comp_name; - first::NothingInt=nothing, last::NothingInt=nothing, - before::NothingSymbol=nothing, after::NothingSymbol=nothing, - reconnect::Bool=true) - - if first !== nothing || last !== nothing - @warn "replace_comp!: Keyword arguments 'first' and 'last' are currently disabled." - first = last = nothing - end +function replace_comp!( + obj::AbstractCompositeComponentDef, + comp_id::ComponentId, + comp_name::Symbol=comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + reconnect::Bool=true +) if ! has_comp(obj, comp_name) error("Cannot replace '$comp_name'; component not found in model.") @@ -894,10 +883,8 @@ function replace_comp!(obj::AbstractCompositeComponentDef, comp_id::ComponentId, end end - # Get original first and last if new run period not specified + # Get the component definition of the component that is being replaced old_comp = compdef(obj, comp_name) - first = first === nothing ? old_comp.first : first - last = last === nothing ? old_comp.last : last if reconnect new_comp = compdef(comp_id) @@ -954,5 +941,5 @@ function replace_comp!(obj::AbstractCompositeComponentDef, comp_id::ComponentId, end # Re-add - return add_comp!(obj, comp_id, comp_name; before=before, after=after) # first=first, last=last, + return add_comp!(obj, comp_id, comp_name; before=before, after=after) end diff --git a/src/core/instances.jl b/src/core/instances.jl index 7f58f06b2..74d9d4f39 100644 --- a/src/core/instances.jl +++ b/src/core/instances.jl @@ -15,15 +15,11 @@ compname(obj::AbstractComponentInstance) = compname(obj.comp_id) """ add_comp!(obj::AbstractCompositeComponentInstance, ci::AbstractComponentInstance) -Add the (leaf or composite) component `ci` to a composite's list of components, and add -the `first` and `last` of `mi` to the ends of the composite's `firsts` and `lasts` lists. +Add the (leaf or composite) component `ci` to a composite's list of components. """ function add_comp!(obj::AbstractCompositeComponentInstance, ci::AbstractComponentInstance) obj.comps_dict[nameof(ci)] = ci - - # push!(obj.firsts, first_period(ci)) # TBD: perhaps this should be set when time is set? - # push!(obj.lasts, last_period(ci)) - nothing + return nothing end # diff --git a/src/core/model.jl b/src/core/model.jl index 6f9f9c690..9a92e3fd3 100644 --- a/src/core/model.jl +++ b/src/core/model.jl @@ -120,15 +120,17 @@ model definition. @delegate update_params!(m::Model, parameters::Dict; update_timesteps = false) => md """ - add_comp!(m::Model, comp_id::ComponentId; comp_name::Symbol=comp_id.comp_name; - first=nothing, last=nothing, before=nothing, after=nothing, rename=nothing) + add_comp!( + m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + rename::NothingPairList=nothing + ) Add the component indicated by `comp_id` to the model indicated by `m`. The component is added at the end of the list unless one of the keywords `before` or `after` is specified. Note that a copy of `comp_id` is made in the composite and assigned the give name. The optional argument `rename` can be a list of pairs indicating `original_name => imported_name`. - -Note: `first` and `last` keywords are currently disabled. """ function add_comp!(m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.comp_name; kwargs...) comp_def = add_comp!(m.md, comp_id, comp_name; kwargs...) @@ -136,25 +138,29 @@ function add_comp!(m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.com end """ - add_comp!(m::Model, comp_def::AbstractComponentDef; comp_name::Symbol=comp_id.comp_name; - first=nothing, last=nothing, before=nothing, after=nothing, rename=nothing) + add_comp!( + m::Model, comp_def::AbstractComponentDef, comp_name::Symbol=comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + rename::NothingPairList=nothing + ) Add the component `comp_def` to the model indicated by `m`. The component is added at the end of the list unless one of the keywords, `first`, `last`, `before`, `after`. Note that a copy of `comp_id` is made in the composite and assigned the give name. The optional argument `rename` can be a list of pairs indicating `original_name => imported_name`. - -Note: `first` and `last` keywords are currently disabled. """ function add_comp!(m::Model, comp_def::AbstractComponentDef, comp_name::Symbol=comp_def.comp_id.comp_name; kwargs...) return add_comp!(m, comp_def.comp_id, comp_name; kwargs...) end """ - replace_comp!(m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.comp_name; - first::NothingSymbol=nothing, last::NothingSymbol=nothing, - before::NothingSymbol=nothing, after::NothingSymbol=nothing, - reconnect::Bool=true) + replace_comp!( + m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.comp_name; + before::NothingSymbol=nothing, + after::NothingSymbol=nothing, + reconnect::Bool=true + ) Replace the component with name `comp_name` in model `m` with the component `comp_id` using the same name. The component is added in the same position as @@ -163,8 +169,6 @@ The component is added with the same first and last values, unless the keywords `first` or `last` are specified. Optional boolean argument `reconnect` with default value `true` indicates whether the existing parameter connections should be maintained in the new component. - -Note: `first` and `last` keywords are currently disabled. """ function replace_comp!(m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.comp_name; kwargs...) comp_def = replace_comp!(m.md, comp_id, comp_name; kwargs...) diff --git a/test/test_components.jl b/test/test_components.jl index bad57e1d5..b08bdde8b 100644 --- a/test/test_components.jl +++ b/test/test_components.jl @@ -143,10 +143,7 @@ ci = compinstance(m, :C) # Get the component instance m = Model() set_dimension!(m, :time, 2000:2100) -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(m, testcomp1, :C; first=2010, last=2090) # Give explicit first and last values for the component -) +add_comp!(m, testcomp1, :C) cd = compdef(m.md, :C) # Get the component definition in the model diff --git a/test/test_datum_storage.jl b/test/test_datum_storage.jl index 2b6151ac3..a484fb0a8 100644 --- a/test/test_datum_storage.jl +++ b/test/test_datum_storage.jl @@ -40,10 +40,7 @@ nregions = length(regions) m = Model() set_dimension!(m, :time, years) -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(m, foo, first=comp_first, last=comp_last) -) +add_comp!(m, foo) run(m) v = m[:foo, :v] @@ -82,10 +79,7 @@ end set_dimension!(m2, :time, years) set_dimension!(m2, :region, regions) -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(m2, baz, first=comp_first, last=comp_last) -) +add_comp!(m2, baz) run(m2) v2 = m2[:baz, :v] @@ -122,10 +116,7 @@ set_dimension!(m, :time, years_variable) end end -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(m, foo2, first=foo2_first, last=foo2_last) -) +add_comp!(m, foo2) run(m) v = m[:foo2, :v] diff --git a/test/test_dimensions.jl b/test/test_dimensions.jl index ce081c563..919ce6e22 100644 --- a/test/test_dimensions.jl +++ b/test/test_dimensions.jl @@ -95,10 +95,7 @@ set_dimension!(m, :time, 2000:2100) # First and last have been disabled... #@test_throws ErrorException add_comp!(m, foo2; first = 2005, last = 2105) # Can't add a component longer than a model -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - foo2_ref = add_comp!(m, foo2; first = 2005, last = 2095) -) +foo2_ref = add_comp!(m, foo2) foo2_ref = ComponentReference(m, :foo2) my_foo2 = compdef(foo2_ref) diff --git a/test/test_getdataframe.jl b/test/test_getdataframe.jl index 0e4bb0ab2..467eb978f 100644 --- a/test/test_getdataframe.jl +++ b/test/test_getdataframe.jl @@ -40,10 +40,7 @@ add_comp!(model1, testcomp1) set_param!(model1, :testcomp1, :par1, years) set_param!(model1, :testcomp1, :par_scalar, 5.) -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(model1, testcomp2; first = late_first, last = early_last) -) +add_comp!(model1, testcomp2) @test_throws ErrorException set_param!(model1, :testcomp2, :par2, late_first:5:early_last) set_param!(model1, :testcomp2, :par2, years) @@ -127,10 +124,7 @@ dim = Mimi.dimension(model3, :time) late_first = 2030 early_last = 2100 -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(model3, testcomp3; first = late_first, last = early_last) -) +add_comp!(model3, testcomp3) indices = collect(late_first:stepsize:early_last) nindices = length(indices) diff --git a/test/test_model_structure_variabletimestep.jl b/test/test_model_structure_variabletimestep.jl index eed91f696..bad489be4 100644 --- a/test/test_model_structure_variabletimestep.jl +++ b/test/test_model_structure_variabletimestep.jl @@ -47,26 +47,10 @@ last_A = 2150 m = Model() set_dimension!(m, :time, years) -# first and last are now disabled -# @test_throws ErrorException add_comp!(m, A, last = 2210) -# @test_throws ErrorException add_comp!(m, A, first = 2010) - -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(m, A, last = 2210) -) - -# remove the comp we just added so later tests succeed -delete!(m, :A) -@test has_comp(m, :A) == false - @test_throws ArgumentError add_comp!(m, A, after=:B) # @test_throws ErrorException add_comp!(m, A, after=:B) -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(m, A, first = first_A, last = last_A) #test specific last and first -) +add_comp!(m, A) add_comp!(m, B, before=:A) diff --git a/test/test_parametertypes.jl b/test/test_parametertypes.jl index e66a5e1fd..3f77a5469 100644 --- a/test/test_parametertypes.jl +++ b/test/test_parametertypes.jl @@ -154,11 +154,7 @@ run(m) m = Model() set_dimension!(m, :time, [2000, 2005, 2020]) - -@test_logs( - (:warn, "add_comp!: Keyword arguments 'first' and 'last' are currently disabled."), - add_comp!(m, MyComp2; first=2000, last=2020) -) +add_comp!(m, MyComp2) set_param!(m, :MyComp2, :x, [1, 2, 3]) set_dimension!(m, :time, [2005, 2020, 2050]) From 29a49b928829e3b63af26600d0c8abbceded566f Mon Sep 17 00:00:00 2001 From: Cora Kingdon Date: Thu, 17 Oct 2019 11:55:03 -0400 Subject: [PATCH 2/2] Remove use of first/last in docs --- docs/src/integrationguide.md | 6 +----- docs/src/userguide.md | 5 +---- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/docs/src/integrationguide.md b/docs/src/integrationguide.md index 07adee874..a51faa303 100644 --- a/docs/src/integrationguide.md +++ b/docs/src/integrationguide.md @@ -58,11 +58,7 @@ In an effort to standardize the function naming protocol within Mimi, and to str Changes to various optional keyword arguments: -- `add_comp!`: Previously the optional keyword arguments `start` and `stop` could be used to specify times for components that do not run for the full length of the model. These arguments are now `first` and `last` respectively. - -```julia -add_comp!(mymodel, ComponentC; first=2010, last=2100) -``` +- `add_comp!`: Through Mimi v0.9.4, the optional keyword arguments `first` and `last` could be used to specify times for components that do not run for the full length of the model, like this: `add_comp!(mymodel, ComponentC; first=2010, last=2100)`. This functionality is currently disabled, and all components must run for the full length of the model's time dimension. This functionality may be re-implemented in a later version of Mimi. ## Running a Model diff --git a/docs/src/userguide.md b/docs/src/userguide.md index d94e05cba..552f9ad42 100644 --- a/docs/src/userguide.md +++ b/docs/src/userguide.md @@ -72,12 +72,9 @@ The next step is to add components to the model. This is done by the following s ```julia add_comp!(mymodel, ComponentA, :GDP) -add_comp!(mymodel, ComponentB; first=2010) -add_comp!(mymodel, ComponentC; first=2010, last=2100) - ``` -The first argument to `add_comp!` is the model, the second is the name of the ComponentId defined by `@defcomp`. If an optional third symbol is provided (as in the first line above), this will be used as the name of the component in this model. This allows you to add multiple versions of the same component to a model, with different names. You can also have components that do not run for the full length of the model. You can specify custom first and last times with the optional keyword arguments as shown above. If no first or last time is provided, the component will assume the first or last time of the model's time index values that were specified in `set_dimension!`. +The first argument to `add_comp!` is the model, the second is the name of the ComponentId defined by `@defcomp`. If an optional third symbol is provided (as in the first line above), this will be used as the name of the component in this model. This allows you to add multiple versions of the same component to a model, with different names. The next step is to set the values for all the parameters in the components. Parameters can either have their values assigned from external data, or they can internally connect to the values from variables in other components of the model.