Potentially change how build handles default values

src/core/build.jl

@@ -338,11 +338,10 @@ end
 
 function build(m::Model)
 
-    # apply defaults to unset parameters
-    _set_defaults!(m.md)
-
     # Reference a copy in the ModelInstance to avoid changes underfoot
     md = deepcopy(m.md)
+    _set_defaults!(md)  # apply defaults to unset parameters in the model instance's copy of the model definition
+
     m.mi = _build(md)
     m.md.dirty = false
     return nothing

test/runtests.jl

@@ -59,6 +59,9 @@ Electron.prep_test_env()
     @info("test_parametertypes.jl")
     @time include("test_parametertypes.jl")
 
+    @info("test_defaults.jl")
+    @time include("test_defaults.jl")
+
     @info("test_marginal_models.jl")
     @time include("test_marginal_models.jl")
 

test/test_composite_parameters.jl

@@ -15,7 +15,7 @@ end
     p1 = Parameter(unit = "thous \$", default=3)
     p3 = Parameter(description="B p3")
     p4 = Parameter(index=[time])
-
+    p5 = Parameter(default = 1)
 end
 
 #------------------------------------------------------------------------------
@@ -134,4 +134,14 @@ set_param!(m2, :B, :p1, :B_p1, 2)   # Use a unique name to set B.p1
 @test length(m2.md.external_param_conns) == 2 
 @test Set(keys(m2.md.external_params)) == Set([:p1, :B_p1])
 
+# Test defaults being set properly:
+m3 = get_model()
+set_param!(m3, :p1, 1, ignoreunits=true)    # Need to set parameter values for all except :p5, which has a default
+set_param!(m3, :p2, 2)    
+set_param!(m3, :p3, 3)    
+set_param!(m3, :p4, 1:10)    
+run(m3)
+@test length(keys(m3.md.external_params)) == 4      # The default value was not added to the original md's list
+@test length(keys(m3.mi.md.external_params)) == 5   # Only added to the model instance's definition
+
 end

test/test_defaults.jl

@@ -0,0 +1,45 @@
+module TestDefaults
+
+using Mimi
+using Test
+
+
+@defcomp A begin
+    p1 = Parameter(default = 1)
+    p2 = Parameter()
+end
+
+m = Model()
+set_dimension!(m, :time, 1:10)
+add_comp!(m, A)
+set_param!(m, :p2, 2)
+
+# So far only :p2 is in the model definition's dictionary
+@test length(m.md.external_params) == 1
+
+run(m)
+
+# During build, :p1's value is set to it's default 
+@test m[:A, :p1] == 1
+
+# But the original model definition does not have :p1 in it's external parameters
+@test length(m.md.external_params) == 1     
+@test length(m.mi.md.external_params) == 2      # But the model instance's md is where the default value was set
+@test ! (:p1 in keys(m.md.external_params))
+@test :p1 in keys(m.mi.md.external_params)
+
+# This errors because p1 isn't in the model definition's external params 
+@test_throws ErrorException update_param!(m, :p1, 10)  
+
+# Need to use set_param! instead
+set_param!(m, :p1, 10)
+
+# Now there is a :p1 in the model definition's dictionary
+@test :p1 in keys(m.md.external_params)
+
+run(m)
+@test m[:A, :p1] == 10
+update_param!(m, :p1, 11)   # Now we can use update_param!
+
+
+end

test/test_parametertypes.jl

@@ -74,10 +74,9 @@ set_param!(m, :MyComp, :e, [1,2,3,4])
 set_param!(m, :MyComp, :f, reshape(1:16, 4, 4))
 set_param!(m, :MyComp, :j, [1,2,3])
 
-build(m)    # applies defaults, creating external params
-extpars = external_params(m)
+build(m)    # applies defaults, creating external params in the model instance's copied definition
+extpars = external_params(m.mi.md)
 
-# TBD: These are not (yet) external params. Defaults are applied at build time.
 @test isa(extpars[:a], ArrayModelParameter)
 @test isa(extpars[:b], ArrayModelParameter)
 
@@ -101,20 +100,22 @@ extpars = external_params(m)
 @test_throws ErrorException update_param!(m, :a, ones(101)) # wrong size
 @test_throws ErrorException update_param!(m, :a, fill("hi", 101, 3)) # wrong type
 
-update_param!(m, :a, Array{Int,2}(zeros(101, 3))) # should be able to convert from Int to Float
-
+set_param!(m, :a, Array{Int,2}(zeros(101, 3))) # should be able to convert from Int to Float
 @test_throws ErrorException update_param!(m, :d, ones(5)) # wrong type; should be scalar
 update_param!(m, :d, 5) # should work, will convert to float
-@test extpars[:d].value == 5
+new_extpars = external_params(m)    # Since there are changes since the last build, need to access the updated dictionary in the model definition
+@test extpars[:d].value == 0.5      # The original dictionary still has the old value
+@test new_extpars[:d].value == 5.   # The new dicitonary has the updated value
 @test_throws ErrorException update_param!(m, :e, 5) # wrong type; should be array
 @test_throws ErrorException update_param!(m, :e, ones(10)) # wrong size
 update_param!(m, :e, [4,5,6,7])
 
-@test length(extpars) == 9
-@test typeof(extpars[:a].values) == TimestepMatrix{FixedTimestep{2000, 1}, arrtype, 1}
+@test length(extpars) == 9          # The old dictionary has the default values that were added during build, so it has more entries
+@test length(new_extpars) == 6
+@test typeof(new_extpars[:a].values) == TimestepMatrix{FixedTimestep{2000, 1}, arrtype, 1}
 
-@test typeof(extpars[:d].value) == numtype
-@test typeof(extpars[:e].values) == Array{arrtype, 1}
+@test typeof(new_extpars[:d].value) == numtype
+@test typeof(new_extpars[:e].values) == Array{arrtype, 1}
 
 
 #------------------------------------------------------------------------------