From d4e6687148392a39815e1f236fc14467f315c10a Mon Sep 17 00:00:00 2001 From: Richard Plevin Date: Tue, 5 Mar 2019 13:40:42 -0800 Subject: [PATCH 1/2] - Eliminated global _compdefs dictionary and changed @defcomp to store the defined ComponentDef rather than storing a ComponentId. - Updated all tests --- src/Mimi.jl | 2 +- src/core/connections.jl | 3 +- src/core/defcomp.jl | 15 +++--- src/core/defs.jl | 70 ++++---------------------- src/core/model.jl | 14 ++++++ test/test_components.jl | 17 ++----- test/test_main.jl | 2 - test/test_main_variabletimestep.jl | 2 - test/test_metainfo.jl | 4 +- test/test_metainfo_variabletimestep.jl | 4 +- 10 files changed, 41 insertions(+), 92 deletions(-) diff --git a/src/Mimi.jl b/src/Mimi.jl index 3481f3fda..951dc91f6 100644 --- a/src/Mimi.jl +++ b/src/Mimi.jl @@ -87,7 +87,7 @@ end # Components are defined here to allow pre-compilation to work function __init__() - compdir = joinpath(dirname(@__FILE__), "components") + compdir = joinpath(@__DIR__, "components") load_comps(compdir) end diff --git a/src/core/connections.jl b/src/core/connections.jl index c5c303ee3..e2b922ab7 100644 --- a/src/core/connections.jl +++ b/src/core/connections.jl @@ -483,8 +483,7 @@ function add_connector_comps(md::ModelDef) end # Fetch the definition of the appropriate connector commponent - conn_name = num_dims == 1 ? :ConnectorCompVector : :ConnectorCompMatrix - conn_comp_def = compdef(conn_name) + conn_comp_def = (num_dims == 1 ? Mimi.ConnectorCompVector : Mimi.ConnectorCompMatrix) conn_comp_name = connector_comp_name(i) # Add the connector component before the user-defined component that required it diff --git a/src/core/defcomp.jl b/src/core/defcomp.jl index f2a016355..477278ef3 100644 --- a/src/core/defcomp.jl +++ b/src/core/defcomp.jl @@ -136,12 +136,14 @@ macro defcomp(comp_name, ex) comp_name = cmpname end - # We'll return a block of expressions that will define the component. First, - # save the ComponentId to a variable with the same name as the component. + # We'll return a block of expressions that will define the component. # @__MODULE__ is evaluated when the expanded macro is interpreted result = :( - let current_module = @__MODULE__ - global const $comp_name = Mimi.ComponentId(nameof(current_module), $(QuoteNode(comp_name))) + let current_module = @__MODULE__, + comp_id = Mimi.ComponentId(nameof(current_module), $(QuoteNode(comp_name))), + comp = Mimi.ComponentDef(comp_id) + + global $comp_name = comp end ) @@ -151,9 +153,6 @@ macro defcomp(comp_name, ex) push!(let_block, expr) end - newcomp = :(comp = new_comp($comp_name, $defcomp_verbosity)) - addexpr(newcomp) - for elt in elements @debug "elt: $elt" @@ -297,7 +296,7 @@ macro defmodel(model_name, ex) addexpr(expr) name = (alias === nothing ? comp_name : alias) - expr = :(add_comp!($model_name, eval(comp_mod_name).$comp_name, $(QuoteNode(name)))) + expr = :(add_comp!($model_name, Mimi.ComponentId(comp_mod_name, $(QuoteNode(comp_name))), $(QuoteNode(name)))) # TBD: extend comp.var syntax to allow module name, e.g., FUND.economy.ygross elseif (@capture(elt, src_comp_.src_name_[arg_] => dst_comp_.dst_name_) || diff --git a/src/core/defs.jl b/src/core/defs.jl index 8cb7731fa..551ed9b4b 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -1,22 +1,4 @@ -# Global component registry: @defcomp stores component definitions here -global const _compdefs = Dict{ComponentId, ComponentDef}() - -compdefs() = collect(values(_compdefs)) - -compdef(comp_id::ComponentId) = _compdefs[comp_id] - -function compdef(comp_name::Symbol) - matches = collect(Iterators.filter(obj -> name(obj) == comp_name, values(_compdefs))) - count = length(matches) - - if count == 1 - return matches[1] - elseif count == 0 - error("Component $comp_name was not found in the global registry") - else - error("Multiple components named $comp_name were found in the global registry") - end -end +compdef(comp_id::ComponentId) = Base.eval(Base.eval(Main, comp_id.module_name), comp_id.comp_name) compdefs(md::ModelDef) = values(md.comp_defs) @@ -27,10 +9,8 @@ hascomp(md::ModelDef, comp_name::Symbol) = haskey(md.comp_defs, comp_name) compdef(md::ModelDef, comp_name::Symbol) = md.comp_defs[comp_name] function reset_compdefs(reload_builtins=true) - empty!(_compdefs) - if reload_builtins - compdir = joinpath(dirname(@__FILE__), "..", "components") + compdir = joinpath(@__DIR__, "..", "components") load_comps(compdir) end end @@ -43,11 +23,14 @@ last_period(md::ModelDef, comp_def::ComponentDef) = last_period(comp_def) === no # Return the module object for the component was defined in compmodule(comp_id::ComponentId) = comp_id.module_name - compname(comp_id::ComponentId) = comp_id.comp_name +compmodule(comp_def::ComponentDef) = compmodule(comp_def.comp_id) +compname(comp_def::ComponentDef) = compname(comp_def.comp_id) + + function Base.show(io::IO, comp_id::ComponentId) - print(io, "$(comp_id.module_name).$(comp_id.comp_name)") + print(io, "") end """ @@ -61,40 +44,6 @@ number_type(md::ModelDef) = md.number_type numcomponents(md::ModelDef) = length(md.comp_defs) - -function dump_components() - for comp in compdefs() - println("\n$(name(comp))") - for (tag, objs) in ((:Variables, variables(comp)), (:Parameters, parameters(comp)), (:Dimensions, dimensions(comp))) - println(" $tag") - for obj in objs - println(" $(obj.name) = $obj") - end - end - end -end - -""" - new_comp(comp_id::ComponentId, verbose::Bool=true) - -Add an empty `ComponentDef` to the global component registry with the given -`comp_id`. The empty `ComponentDef` must be populated with calls to `addvariable`, -`addparameter`, etc. -""" -function new_comp(comp_id::ComponentId, verbose::Bool=true) - if verbose - if haskey(_compdefs, comp_id) - @warn "Redefining component $comp_id" - else - @info "new component $comp_id" - end - end - - comp_def = ComponentDef(comp_id) - _compdefs[comp_id] = comp_def - return comp_def -end - """ delete!(m::ModelDef, component::Symbol @@ -529,13 +478,14 @@ function _add_anonymous_dims!(md::ModelDef, comp_def::ComponentDef) end """ - add_comp!(md::ModelDef, comp_def::ComponentDef; first=nothing, last=nothing, before=nothing, after=nothing) + add_comp!(md::ModelDef, comp_def::ComponentDef, comp_name::Symbol=comp_def.comp_id.comp_name; + first=nothing, last=nothing, before=nothing, after=nothing) Add the component indicated by `comp_def` to the model indcated by `md`. The component is added at the end of the list unless one of the keywords, `first`, `last`, `before`, `after`. If the `comp_name` differs from that in the `comp_def`, a copy of `comp_def` is made and assigned the new name. """ -function add_comp!(md::ModelDef, comp_def::ComponentDef, comp_name::Symbol; +function add_comp!(md::ModelDef, comp_def::ComponentDef, comp_name::Symbol=comp_def.comp_id.comp_name; first::NothingInt=nothing, last::NothingInt=nothing, before::NothingSymbol=nothing, after::NothingSymbol=nothing) diff --git a/src/core/model.jl b/src/core/model.jl index 6a93d09a7..b32a3d40b 100644 --- a/src/core/model.jl +++ b/src/core/model.jl @@ -160,6 +160,13 @@ function add_comp!(m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.com return ComponentReference(m, comp_name) end +function add_comp!(m::Model, comp_def::ComponentDef, comp_name::Symbol=comp_def.comp_id.comp_name; + first=nothing, last=nothing, before=nothing, after=nothing) + add_comp!(m.md, comp_def, comp_name; first=first, last=last, before=before, after=after) + decache(m) + return ComponentReference(m, comp_name) +end + """ replace_comp!(m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id.comp_name; first::NothingSymbol=nothing, last::NothingSymbol=nothing, @@ -183,6 +190,13 @@ function replace_comp!(m::Model, comp_id::ComponentId, comp_name::Symbol=comp_id return ComponentReference(m, comp_name) end +function replace_comp!(m::Model, comp_def::ComponentDef, comp_name::Symbol=comp_def.comp_id.comp_name; + first::NothingSymbol=nothing, last::NothingSymbol=nothing, + before::NothingSymbol=nothing, after::NothingSymbol=nothing, + reconnect::Bool=true) + replace_comp!(m, comp_def.comp_id, comp_name; first=first, last=last, before=before, after=after, reconnect=reconnect) +end + """ components(m::Model) diff --git a/test/test_components.jl b/test/test_components.jl index 40100e91a..0daf0bdb0 100644 --- a/test/test_components.jl +++ b/test/test_components.jl @@ -4,14 +4,12 @@ using Mimi using Test import Mimi: - reset_compdefs, compdefs, compdef, compkeys, hascomp, _compdefs, first_period, - last_period, compmodule, compname, numcomponents, dump_components, + reset_compdefs, compdefs, compdef, compkeys, hascomp, first_period, + last_period, compmodule, compname, numcomponents, dim_keys, dim_values, dimensions reset_compdefs() -@test length(_compdefs) == 3 # adder, ConnectorCompVector, ConnectorCompMatrix - my_model = Model() # Try running model with no components @@ -81,8 +79,7 @@ comps = collect(compdefs(my_model)) # Test compdefs, compdef, compkeys, etc. @test comps == collect(compdefs(my_model.md)) @test length(comps) == 3 -@test compdef(:testcomp3) == comps[3] -@test_throws ErrorException compdef(:testcomp4) #this component does not exist +@test testcomp3 == comps[3] @test [compkeys(my_model.md)...] == [:testcomp1, :testcomp2, :testcomp3] @test hascomp(my_model.md, :testcomp1) == true && hascomp(my_model.md, :testcomp4) == false @@ -93,12 +90,6 @@ comps = collect(compdefs(my_model)) add_comp!(my_model, testcomp3, :testcomp3_v2) @test numcomponents(my_model) == 4 -# Test reset_compdefs methods -reset_compdefs() -@test length(_compdefs) == 3 # adder, ConnectorCompVector, ConnectorCompMatrix -reset_compdefs(false) -@test length(_compdefs) == 0 - #------------------------------------------------------------------------------ # Tests for component run periods when resetting the model's time dimension @@ -115,7 +106,7 @@ end # 1. Test resetting the time dimension without explicit first/last values -cd = compdef(testcomp1) +cd = testcomp1 @test cd.first === nothing # original component definition's first and last values are unset @test cd.last === nothing diff --git a/test/test_main.jl b/test/test_main.jl index c5c78169f..dfc37b63f 100644 --- a/test/test_main.jl +++ b/test/test_main.jl @@ -38,8 +38,6 @@ end foo1.par1 = 5.0 end -@test length(compdefs()) == 4 # adder, 2 connectors, and foo1 - # x1 = foo1(Float64, Dict{Symbol, Int}(:time=>10, :index1=>3)) # x1 = foo1(Float64, Val{1}, Val{1}, Val{10}, Val{1}, Val{1}, Val{1}, Val{1}, Dict{Symbol, Int}(:time=>10, :index1=>3)) diff --git a/test/test_main_variabletimestep.jl b/test/test_main_variabletimestep.jl index ace8f6663..32dba20e6 100644 --- a/test/test_main_variabletimestep.jl +++ b/test/test_main_variabletimestep.jl @@ -38,8 +38,6 @@ end # foo1.par2 = [] end -@test length(compdefs()) == 4 # adder, 2 connectors, and foo1 - # x1 = foo1(Float64, Dict{Symbol, Int}(:time=>10, :index1=>3)) # x1 = foo1(Float64, Val{1}, Val{1}, Val{10}, Val{1}, Val{1}, Val{1}, Val{1}, Dict{Symbol, Int}(:time=>10, :index1=>3)) diff --git a/test/test_metainfo.jl b/test/test_metainfo.jl index a30707a05..62858113f 100644 --- a/test/test_metainfo.jl +++ b/test/test_metainfo.jl @@ -40,8 +40,8 @@ end c1 = compdef(test_model, :ch4forcing1) c2 = compdef(test_model, :ch4forcing2) -@test c1 == compdef(:ch4forcing1) -@test_throws ErrorException compdef(:missingcomp) +@test c1 == compdef(test_model, :ch4forcing1) +@test_throws KeyError compdef(test_model, :missingcomp) @test c2.comp_id.module_name == :TestMetaInfo @test c2.comp_id.comp_name == :ch4forcing1 diff --git a/test/test_metainfo_variabletimestep.jl b/test/test_metainfo_variabletimestep.jl index baebe2954..fd742b6bf 100644 --- a/test/test_metainfo_variabletimestep.jl +++ b/test/test_metainfo_variabletimestep.jl @@ -40,8 +40,8 @@ end c1 = compdef(test_model, :ch4forcing1) c2 = compdef(test_model, :ch4forcing2) -@test c1 == compdef(:ch4forcing1) -@test_throws ErrorException compdef(:missingcomp) +@test c1 == ch4forcing1 +@test_throws KeyError compdef(test_model, :missingcomp) @test c2.comp_id.module_name == :TestMetaInfo_VariableTimestep @test c2.comp_id.comp_name == :ch4forcing1 From 84eb98fb0efbbbf9783f4436bc2830687e8f17b0 Mon Sep 17 00:00:00 2001 From: Richard Plevin Date: Tue, 5 Mar 2019 15:40:50 -0800 Subject: [PATCH 2/2] - Replaced most evals with getfield(module, symbol) - Replaced others with Base.eval(Main, expr) --- src/core/defcomp.jl | 12 ++++++------ src/core/defs.jl | 2 +- src/core/types.jl | 5 ++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/core/defcomp.jl b/src/core/defcomp.jl index 477278ef3..e36149148 100644 --- a/src/core/defcomp.jl +++ b/src/core/defcomp.jl @@ -240,12 +240,14 @@ macro defcomp(comp_name, ex) @debug " index $(Tuple(dimensions)), unit '$unit', desc '$desc'" - dflt = eval(dflt) - if (dflt !== nothing && length(dimensions) != ndims(dflt)) - error("Default value has different number of dimensions ($(ndims(dflt))) than parameter '$name' ($(length(dimensions)))") + if dflt !== nothing + dflt = Base.eval(Main, dflt) + if length(dimensions) != ndims(dflt) + error("Default value has different number of dimensions ($(ndims(dflt))) than parameter '$name' ($(length(dimensions)))") + end end - vartype = vartype === nothing ? Number : eval(vartype) + vartype = (vartype === nothing ? Number : Base.eval(Main, vartype)) addexpr(_generate_var_or_param(elt_type, name, vartype, dimensions, dflt, desc, unit)) else @@ -253,9 +255,7 @@ macro defcomp(comp_name, ex) end end - # addexpr(:($comp_name)) addexpr(:(nothing)) # reduces noise - return esc(result) end diff --git a/src/core/defs.jl b/src/core/defs.jl index 551ed9b4b..182386d04 100644 --- a/src/core/defs.jl +++ b/src/core/defs.jl @@ -1,4 +1,4 @@ -compdef(comp_id::ComponentId) = Base.eval(Base.eval(Main, comp_id.module_name), comp_id.comp_name) +compdef(comp_id::ComponentId) = getfield(getfield(Main, comp_id.module_name), comp_id.comp_name) compdefs(md::ModelDef) = values(md.comp_defs) diff --git a/src/core/types.jl b/src/core/types.jl index 6cf95aa2f..2860a679f 100644 --- a/src/core/types.jl +++ b/src/core/types.jl @@ -357,16 +357,15 @@ mutable struct ComponentInstance{TV <: ComponentInstanceVariables, TP <: Compone comp_name = comp_id.comp_name module_name = comp_id.module_name - comp_module = Base.eval(Main, module_name) + comp_module = getfield(Main, module_name) # TBD: use FunctionWrapper here? function get_func(name) func_name = Symbol("$(name)_$(comp_name)") try - Base.eval(comp_module, func_name) + getfield(comp_module, func_name) catch err # No need to warn about this... - # @warn "Failed to evaluate function name $func_name in module $comp_module" nothing end end