From 67d020af026ca046417aebf50b8b68d494edc5c1 Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Sun, 7 Mar 2021 12:07:16 -0800
Subject: [PATCH 01/10] Change backup size requireemnts to match model time;
 Change keyword arg from offset to backup_offst

---
 docs/src/ref/ref_structures_definitions.md |  2 +-
 src/core/build.jl                          |  2 +-
 src/core/connections.jl                    | 82 +++++++++++++---------
 src/core/defs.jl                           | 11 +--
 src/core/model.jl                          | 24 ++++---
 src/core/paths.jl                          |  2 +-
 src/core/types/params.jl                   |  7 +-
 test/test_connectorcomp.jl                 | 10 +--
 8 files changed, 78 insertions(+), 62 deletions(-)

diff --git a/docs/src/ref/ref_structures_definitions.md b/docs/src/ref/ref_structures_definitions.md
index a23a9cdce..c2ed860c4 100644
--- a/docs/src/ref/ref_structures_definitions.md
+++ b/docs/src/ref/ref_structures_definitions.md
@@ -122,7 +122,7 @@ dst_comp_path::ComponentPath
 dst_par_name::Symbol
 ignoreunits::Bool
 backup::Union{Symbol, Nothing} # a Symbol identifying the external param providing backup data, or nothing
-offset::Int
+backup_offset::Union{Int, Nothing}
 
 # ExternalParameterConnection  <: AbstractConnection
 comp_path::ComponentPath
diff --git a/src/core/build.jl b/src/core/build.jl
index d29601707..e85c79f3a 100644
--- a/src/core/build.jl
+++ b/src/core/build.jl
@@ -175,7 +175,7 @@ function _get_leaf_level_ipcs(md::ModelDef, conn::InternalParameterConnection)
     var_path = ComponentPath(top_src_path, var_sub_path[1])
 
     ipcs = [InternalParameterConnection(var_path, var_name[1], param_path, param_name, 
-        conn.ignoreunits, conn.backup; offset=conn.offset) for (param_path, param_name) in
+        conn.ignoreunits, conn.backup; backup_offset=conn.backup_offset) for (param_path, param_name) in
         zip(param_paths, param_names)]
     return ipcs
 end
diff --git a/src/core/connections.jl b/src/core/connections.jl
index 52231b364..f9ee730f7 100644
--- a/src/core/connections.jl
+++ b/src/core/connections.jl
@@ -113,20 +113,22 @@ end
         dst_comp_path::ComponentPath, dst_par_name::Symbol,
         src_comp_path::ComponentPath, src_var_name::Symbol,
         backup::Union{Nothing, Array}=nothing;
-        ignoreunits::Bool=false, offset::Int=0)
+        ignoreunits::Bool=false, backup_offset::Int=0)
 
 Bind the parameter `dst_par_name` of one component `dst_comp_path` of composite `obj` to a
 variable `src_var_name` in another component `src_comp_path` of the same model using
 `backup` to provide default values and the `ignoreunits` flag to indicate the need to
-check match units between the two.  The `offset` argument indicates the offset between
-the destination and the source ie. the value would be `1` if the destination component
-parameter should only be calculated for the second timestep and beyond.
+check match units between the two.  The `backup_offset` argument, which is only valid 
+when `backup` data has been set, indicates that the backup data should be used for
+a specified number of timesteps after the source component begins. ie. the value would be 
+`1` if the destination componentm parameter should only use the source component 
+data for the second timestep and beyond.
 """
 function _connect_param!(obj::AbstractCompositeComponentDef,
                         dst_comp_path::ComponentPath, dst_par_name::Symbol,
                         src_comp_path::ComponentPath, src_var_name::Symbol,
                         backup::Union{Nothing, Array}=nothing;
-                        ignoreunits::Bool=false, offset::Int=0)
+                        ignoreunits::Bool=false, backup_offset::Union{Nothing, Int}=nothing)
 
     dst_comp_def = compdef(obj, dst_comp_path)
     src_comp_def = compdef(obj, src_comp_path)
@@ -152,33 +154,31 @@ function _connect_param!(obj::AbstractCompositeComponentDef,
                   "Expected size $(datum_size(obj, dst_comp_def, dst_par_name)) but got $(size(backup)).")
         end
 
-        # some other check for second dimension??
-        dst_param = parameter(dst_comp_def, dst_par_name)
-        dst_dims  = dim_names(dst_param)
-
         # convert number type and, if it's a NamedArray, convert to Array
         backup = convert(Array{Union{Missing, number_type(obj)}}, backup)
-        first = first_period(obj, dst_comp_def)
-
-        T = eltype(backup)
 
+        dst_param = parameter(dst_comp_def, dst_par_name)
+        dst_dims  = dim_names(dst_param)
         dim_count = length(dst_dims)
 
-        if dim_count == 0
+        ti = get_time_index_position(dst_param)
+
+        if ti === nothing # not time dimension
             values = backup
-        else
-            ti = get_time_index_position(dst_param)
+        else # handle time dimension
+
+            # get first and last of the ModelDef, NOT the ComponentDef
+            first = first_period(obj)
+            last = last_period(obj) 
+
+            T = eltype(backup)
 
             if isuniform(obj)
-                # use the first from the comp_def not the ModelDef
                 stepsize = step_size(obj)
-                last = last_period(obj, dst_comp_def)
                 values = TimestepArray{FixedTimestep{first, stepsize, last}, T, dim_count, ti}(backup)
             else
                 times = time_labels(obj)
-                # use the first from the comp_def
-                first_index = findfirst(isequal(first), times)
-                values = TimestepArray{VariableTimestep{(times[first_index:end]...,)}, T, dim_count, ti}(backup)
+                values = TimestepArray{VariableTimestep{(times...,)}, T, dim_count, ti}(backup)
             end
 
         end
@@ -187,6 +187,11 @@ function _connect_param!(obj::AbstractCompositeComponentDef,
         backup_param_name = dst_par_name
 
     else
+        # cannot use backup_offset keyword argument if there is no backup
+        if backup_offset !== nothing
+            error("Cannot set `backup_offset` keyword argument if `backup` data is not explicitly provided")
+        end
+
         # If backup not provided, make sure the source component covers the span of the destination component
         src_first, src_last = first_and_last(src_comp_def)
         dst_first, dst_last = first_and_last(dst_comp_def)
@@ -212,7 +217,7 @@ Try calling:
     end
 
     conn = InternalParameterConnection(src_comp_path, src_var_name, dst_comp_path, dst_par_name,
-                                       ignoreunits, backup_param_name, offset=offset)
+                                       ignoreunits, backup_param_name, backup_offset=backup_offset)
     add_internal_param_conn!(obj, conn)
 
     return nothing
@@ -221,29 +226,33 @@ end
 function connect_param!(obj::AbstractCompositeComponentDef,
                         dst_comp_name::Symbol, dst_par_name::Symbol,
                         src_comp_name::Symbol, src_var_name::Symbol,
-                        backup::Union{Nothing, Array}=nothing; ignoreunits::Bool=false, offset::Int=0)
+                        backup::Union{Nothing, Array}=nothing; ignoreunits::Bool=false, 
+                        backup_offset::Union{Nothing, Int} = nothing)
     _connect_param!(obj, ComponentPath(obj, dst_comp_name), dst_par_name,
                         ComponentPath(obj, src_comp_name), src_var_name,
-                        backup; ignoreunits=ignoreunits, offset=offset)
+                        backup; ignoreunits=ignoreunits, backup_offset=backup_offset)
 end
 
 """
     connect_param!(obj::AbstractCompositeComponentDef,
         dst::Pair{Symbol, Symbol}, src::Pair{Symbol, Symbol},
         backup::Union{Nothing, Array}=nothing;
-        ignoreunits::Bool=false, offset::Int=0)
+        ignoreunits::Bool=false, backup_offset::Union{Nothing, Int} = nothing)
 
 Bind the parameter `dst[2]` of one component `dst[1]` of composite `obj`
 to a variable `src[2]` in another component `src[1]` of the same composite
 using `backup` to provide default values and the `ignoreunits` flag to indicate the need
-to check match units between the two.  The `offset` argument indicates the offset
-between the destination and the source ie. the value would be `1` if the destination
-component parameter should only be calculated for the second timestep and beyond.
+to check match units between the two.  The `backup_offset` argument, which is only valid 
+when `backup` data has been set, indicates that the backup data should be used for
+a specified number of timesteps after the source component begins. ie. the value would be 
+`1` if the destination componentm parameter should only use the source component 
+data for the second timestep and beyond.
 """
 function connect_param!(obj::AbstractCompositeComponentDef,
                         dst::Pair{Symbol, Symbol}, src::Pair{Symbol, Symbol},
-                        backup::Union{Nothing, Array}=nothing; ignoreunits::Bool=false, offset::Int=0)
-    connect_param!(obj, dst[1], dst[2], src[1], src[2], backup; ignoreunits=ignoreunits, offset=offset)
+                        backup::Union{Nothing, Array}=nothing; ignoreunits::Bool=false, 
+                        backup_offset::Union{Nothing, Int} = nothing)
+    connect_param!(obj, dst[1], dst[2], src[1], src[2], backup; ignoreunits=ignoreunits, backup_offset=backup_offset)
 end
 
 """
@@ -606,7 +615,9 @@ function add_connector_comps!(obj::AbstractCompositeComponentDef)
             conn_comp_name = connector_comp_name(i) # generate a new name
             i += 1 # increment connector comp counter
 
-            # Add the connector component before the user-defined component that required it
+            # Add the connector component before the user-defined component that 
+            # required it, and for now let the first and last of the component 
+            # be free and thus be set to the same as the model
             conn_comp = add_comp!(obj, conn_comp_def, conn_comp_name, before=comp_name)
             conn_path = conn_comp.comp_path
 
@@ -624,10 +635,15 @@ function add_connector_comps!(obj::AbstractCompositeComponentDef)
             add_external_param_conn!(obj, ExternalParameterConnection(conn_path, :input2, conn.backup))
 
             # set the first and last parameters for WITHIN the component which 
-            # decide when backup is used and when connectin is used
+            # decide when backup is used and when connection is used
             src_comp_def = compdef(obj, conn.src_comp_path)
-            set_param!(obj, conn_comp_name, :first, first_period(obj, src_comp_def) + conn.offset)
-            set_param!(obj, conn_comp_name, :last, last_period(obj, src_comp_def))
+
+            param_last = last_period(obj, src_comp_def)
+            param_first = first_period(obj, src_comp_def)
+            conn.backup_offset !== nothing ? param_first = param_first + conn.backup_offset : nothing
+
+            set_param!(obj, conn_comp_name, :first, param_first)
+            set_param!(obj, conn_comp_name, :last, param_last)
         end
     end
 
diff --git a/src/core/defs.jl b/src/core/defs.jl
index efafdd47f..2c60434bc 100644
--- a/src/core/defs.jl
+++ b/src/core/defs.jl
@@ -251,16 +251,11 @@ function check_parameter_dimensions(md::ModelDef, value::AbstractArray, dims::Ve
     end
 end
 
-# TBD: is this needed for composites?
+# we now require the backup data to have the same dimensions as the model, regardless
+# of the time span of the specific component
 function datum_size(obj::AbstractCompositeComponentDef, comp_def::AbstractComponentDef, datum_name::Symbol)
     dims = dim_names(comp_def, datum_name)
-    if dims[1] == :time
-        time_length = getspan(obj, comp_def)[1]
-        rest_dims = filter(x->x!=:time, dims)
-        datum_size = (time_length, dim_counts(obj, rest_dims)...,)
-    else
-        datum_size = (dim_counts(obj, dims)...,)
-    end
+    datum_size = (dim_counts(obj, dims)...,)
     return datum_size
 end
 
diff --git a/src/core/model.jl b/src/core/model.jl
index a07594fad..56a9e4835 100644
--- a/src/core/model.jl
+++ b/src/core/model.jl
@@ -36,20 +36,22 @@ is_built(mm::MarginalModel) = (is_built(mm.base) && is_built(mm.modified))
 
 """
     connect_param!(m::Model, dst_comp_name::Symbol, dst_par_name::Symbol, src_comp_name::Symbol, src_var_name::Symbol, 
-    backup::Union{Nothing, Array}=nothing; ignoreunits::Bool=false, offset::Int=0)
+    backup::Union{Nothing, Array}=nothing; ignoreunits::Bool=false, backup_offset::Union{Int, Nothing}=nothing)
 
 Bind the parameter `dst_par_name` of one component `dst_comp_name` of model `m`
 to a variable `src_var_name` in another component `src_comp_name` of the same model
 using `backup` to provide default values and the `ignoreunits` flag to indicate the need
-to check match units between the two.  The `offset` argument indicates the offset
-between the destination and the source ie. the value would be `1` if the destination
-component parameter should only be calculated for the second timestep and beyond.
+to check match units between the two.  The `backup_offset` argument, which is only valid 
+when `backup` data has been set, indicates that the backup data should be used for
+a specified number of timesteps after the source component begins. ie. the value would be 
+`1` if the destination componentm parameter should only use the source component 
+data for the second timestep and beyond.
 """
 @delegate connect_param!(m::Model,
                          dst_comp_name::Symbol, dst_par_name::Symbol,
                          src_comp_name::Symbol, src_var_name::Symbol,
                          backup::Union{Nothing, Array}=nothing;
-                         ignoreunits::Bool=false, offset::Int=0) => md
+                         ignoreunits::Bool=false, backup_offset::Union{Int, Nothing} = nothing) => md
 
 """
     connect_param!(m::Model, comp_name::Symbol, param_name::Symbol, ext_param_name::Symbol)
@@ -65,15 +67,17 @@ Bind the parameter `param_name` in the component `comp_name` of model `m` to the
 Bind the parameter `dst[2]` of one component `dst[1]` of model `m`
 to a variable `src[2]` in another component `src[1]` of the same model
 using `backup` to provide default values and the `ignoreunits` flag to indicate the need
-to check match units between the two.  The `offset` argument indicates the offset
-between the destination and the source ie. the value would be `1` if the destination
-component parameter should only be calculated for the second timestep and beyond.
+to check match units between the two.  The `backup_offset` argument, which is only valid 
+when `backup` data has been set, indicates that the backup data should be used for
+a specified number of timesteps after the source component begins. ie. the value would be 
+`1` if the destination componentm parameter should only use the source component 
+data for the second timestep and beyond.
 
 """
 function connect_param!(m::Model, dst::Pair{Symbol, Symbol}, src::Pair{Symbol, Symbol},
                            backup::Union{Nothing, Array}=nothing;
-                           ignoreunits::Bool=false, offset::Int=0)
-    connect_param!(m.md, dst[1], dst[2], src[1], src[2], backup; ignoreunits=ignoreunits, offset=offset)
+                           ignoreunits::Bool=false, backup_offset::Union{Int, Nothing} = nothing)
+    connect_param!(m.md, dst[1], dst[2], src[1], src[2], backup; ignoreunits=ignoreunits, backup_offset=backup_offset)
 end
 
 """
diff --git a/src/core/paths.jl b/src/core/paths.jl
index a85eb95d8..b6cff3773 100644
--- a/src/core/paths.jl
+++ b/src/core/paths.jl
@@ -56,7 +56,7 @@ function _fix_comp_path!(child::AbstractComponentDef, parent::AbstractCompositeC
             conns[i] = InternalParameterConnection(src_path, conn.src_var_name,
                                                    dst_path, conn.dst_par_name,
                                                    conn.ignoreunits, conn.backup;
-                                                   offset=conn.offset)
+                                                   backup_offset=conn.backup_offset)
         end
 
     else
diff --git a/src/core/types/params.jl b/src/core/types/params.jl
index da70b8b04..deb48c135 100644
--- a/src/core/types/params.jl
+++ b/src/core/types/params.jl
@@ -58,12 +58,13 @@ struct InternalParameterConnection <: AbstractConnection
     dst_par_name::Symbol
     ignoreunits::Bool
     backup::Union{Symbol, Nothing} # a Symbol identifying the external param providing backup data, or nothing
-    offset::Int
+    backup_offset::Union{Int, Nothing}
 
     function InternalParameterConnection(src_path::ComponentPath, src_var::Symbol,
                                          dst_path::ComponentPath, dst_par::Symbol,
-                                         ignoreunits::Bool, backup::Union{Symbol, Nothing}=nothing; offset::Int=0)
-        self = new(src_path, src_var, dst_path, dst_par, ignoreunits, backup, offset)
+                                         ignoreunits::Bool, backup::Union{Symbol, Nothing}=nothing;
+                                         backup_offset::Union{Symbol, Nothing}=nothing)
+        self = new(src_path, src_var, dst_path, dst_par, ignoreunits, backup, backup_offset)
         return self
     end
 end
diff --git a/test/test_connectorcomp.jl b/test/test_connectorcomp.jl
index 5c6850a42..628a52745 100644
--- a/test/test_connectorcomp.jl
+++ b/test/test_connectorcomp.jl
@@ -3,7 +3,7 @@ module TestConnectorComp
 using Mimi
 using Test
 
-import Mimi: compdef, compdefs, components
+import Mimi: compdef, compdefs, components, getspan
 
 @defcomp Long begin
     x = Parameter(index=[time])
@@ -206,12 +206,12 @@ late_start_long = 2002
 model5 = Model()
 set_dimension!(model5, :time, years)
 add_comp!(model5, Short; first = late_start)
-add_comp!(model5, Long; first = late_start_long)    # starts later as well, so backup data needs to match this size
+add_comp!(model5, Long; first = late_start_long)    
 set_param!(model5, :Short, :a, 2)
 
-# A. test wrong size (needs to be length of component, not length of model)
-@test_throws ErrorException connect_param!(model5, :Long=>:x, :Short=>:b, zeros(length(years)))
-@test_throws ErrorException connect_param!(model4, :Long_multi=>:x, :Short_multi=>:b, zeros(length(years), length(regions)+1)) # test case with >1 dimension
+# A. test wrong size (needs to be length of model, not length of component)
+@test_throws ErrorException connect_param!(model5, :Long=>:x, :Short=>:b, zeros(getspan(model5, :Long)))
+@test_throws ErrorException connect_param!(model4, :Long_multi=>:x, :Short_multi=>:b, zeros(getspan(model4, :Long_multi)[1], length(regions)+1)) # test case with >1 dimension
 
 
 # B. test no backup data provided

From 2f41320b35312db1c88f1867e192b1cc96108409 Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Sun, 7 Mar 2021 14:55:06 -0800
Subject: [PATCH 02/10] Fix bug

---
 src/core/types/params.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/types/params.jl b/src/core/types/params.jl
index deb48c135..9769a8eb0 100644
--- a/src/core/types/params.jl
+++ b/src/core/types/params.jl
@@ -63,7 +63,7 @@ struct InternalParameterConnection <: AbstractConnection
     function InternalParameterConnection(src_path::ComponentPath, src_var::Symbol,
                                          dst_path::ComponentPath, dst_par::Symbol,
                                          ignoreunits::Bool, backup::Union{Symbol, Nothing}=nothing;
-                                         backup_offset::Union{Symbol, Nothing}=nothing)
+                                         backup_offset::Union{Int, Nothing}=nothing)
         self = new(src_path, src_var, dst_path, dst_par, ignoreunits, backup, backup_offset)
         return self
     end

From 5f900d4ed43700f479aaa1bf2d58b132b13015a3 Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Sun, 7 Mar 2021 20:29:46 -0800
Subject: [PATCH 03/10] Add tests

---
 test/test_connectorcomp.jl | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/test/test_connectorcomp.jl b/test/test_connectorcomp.jl
index 628a52745..38f2186ea 100644
--- a/test/test_connectorcomp.jl
+++ b/test/test_connectorcomp.jl
@@ -51,7 +51,6 @@ x = model1[:Long, :x]
 @test length(x) == length(years)
 
 @test all(ismissing, b[1:year_dim[late_start]-1])
-
 @test all(iszero, x[1:year_dim[late_start]-1])
 
 # Test the values are right after the late start
@@ -65,6 +64,17 @@ offset = late_start - years[1]
 b = getdataframe(model1, :Short, :b)
 @test size(b) == (length(years), 2)
 
+# Try connecting using backup_offset
+connect_param!(model1, :Long, :x, :Short, :b, zeros(length(years)), backup_offset = 1)
+run(model1)
+
+b = model1[:Short, :b]
+x = model1[:Long, :x]
+
+@test all(ismissing, b[1:year_dim[late_start]-1])
+@test all(iszero, x[1:year_dim[late_start]]) # one more zero because used backup an extra year
+@test b[year_dim[late_start]+1:end] == x[year_dim[late_start]+1:end] # the rest of the data shoudl match
+
 #------------------------------------------------------------------------------
 #  2. Test with a short component that ends early (and test Variable timesteps)
 #------------------------------------------------------------------------------
@@ -217,7 +227,6 @@ set_param!(model5, :Short, :a, 2)
 # B. test no backup data provided
 @test_throws ErrorException connect_param!(model5, :Long=>:x, :Short=>:b)   # Error because no backup data provided
 
-
 #------------------------------------------------------------------------------
 #  6. Test connecting Short component to Long component (does not add a
 #       connector component)
@@ -235,6 +244,7 @@ model6 = Model()
 set_dimension!(model6, :time, years)
 add_comp!(model6, foo, :Long; rename=[:var => :long_foo])
 add_comp!(model6, foo, :Short; rename=[:var => :short_foo],first=late_start)
+@test_throws ErrorException connect_param!(model6, :Short => :par, :Long => :var, backup_offset = 1) # can't use backup_offset without backup
 connect_param!(model6, :Short => :par, :Long => :var)
 set_param!(model6, :Long, :par, years)
 

From 1fc2558fff3ffb5dbf0f4d67a9ea33fdbfaf3e30 Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Sun, 7 Mar 2021 22:21:44 -0800
Subject: [PATCH 04/10] Start adding first and last to composite components

---
 src/core/defcomposite.jl |  29 +++++++---
 test/test_firstlast.jl   | 111 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+), 6 deletions(-)

diff --git a/src/core/defcomposite.jl b/src/core/defcomposite.jl
index c071f6718..dd328302c 100644
--- a/src/core/defcomposite.jl
+++ b/src/core/defcomposite.jl
@@ -78,22 +78,39 @@ end
 # Convert @defcomposite "shorthand" statements into Mimi API calls
 #
 function _parse(expr)
-    valid_keys = (:default, :description, :unit)
+
     result = nothing
+    
+    if @capture(expr, newname_ = Component(compname_, args__)) ||
+        @capture(expr, Component(compname_, args__))
+
+        valid_keys = (:first, :last)
 
-    if @capture(expr, newname_ = Component(compname_)) ||
-       @capture(expr, Component(compname_))
         # check newname is nothing or Symbol, compname is Symbol
         _typecheck(compname, Symbol, "Referenced component name")
-
         if newname !== nothing
             _typecheck(newname, Symbol, "Local name for component name")
         end
-
+        
+        #assign newname
         newname = (newname === nothing ? compname : newname)
-        result = :(Mimi.add_comp!(obj, $compname, $(QuoteNode(newname))))
+        
+        # handle keyword arguments
+        keyargs = []        
+        for arg in args
+            @capture(arg, keywd_ = value_) # keyword arguments
+            if keywd in valid_keys
+                push!(keyargs, arg)
+            else
+                error("Unrecognized Component keyword '$keywd'; must be 'first' or 'last")
+            end
+        end
+
+        result = :(Mimi.add_comp!(obj, $compname, $(QuoteNode(newname)); $(keyargs...)))
 
     elseif @capture(expr, localparname_ = Parameter(args__))
+        valid_keys = (:default, :description, :unit)
+
         regargs = []
         keyargs = []
 
diff --git a/test/test_firstlast.jl b/test/test_firstlast.jl
index 1b07f942f..248c0eca5 100644
--- a/test/test_firstlast.jl
+++ b/test/test_firstlast.jl
@@ -311,4 +311,115 @@ for year in collect(1995:1999)
 	@test m[:MyComp, :a][5] == 1.0
 end
 
+# test composite
+
+@defcomp Comp1 begin
+    par_1_1 = Parameter(index=[time])      # external input
+    var_1_1 = Variable(index=[time])       # computed
+    foo = Parameter()
+
+    function run_timestep(p, v, d, t)
+        v.var_1_1[t] = p.par_1_1[t]
+    end
+end
+
+@defcomp Comp2 begin
+    par_2_1 = Parameter(index=[time])      # connected to Comp1.var_1_1
+    par_2_2 = Parameter(index=[time])      # external input
+    var_2_1 = Variable(index=[time])       # computed
+    foo = Parameter()
+
+    function run_timestep(p, v, d, t)
+        v.var_2_1[t] = p.par_2_1[t] + p.foo * p.par_2_2[t]
+    end
+end
+
+@defcomp Comp3 begin
+    par_3_1 = Parameter(index=[time])      # connected to Comp2.var_2_1
+    var_3_1 = Variable(index=[time])       # external output
+    foo = Parameter(default=30)
+
+    function run_timestep(p, v, d, t)
+        # @info "Comp3 run_timestep"
+        v.var_3_1[t] = p.par_3_1[t] * 2
+    end
+end
+
+@defcomp Comp4 begin
+    par_4_1 = Parameter(index=[time])      # connected to Comp2.var_2_1
+    var_4_1 = Variable(index=[time])        # external output
+    foo = Parameter(default=300)
+
+    function run_timestep(p, v, d, t)
+        # @info "Comp4 run_timestep"
+        v.var_4_1[t] = p.par_4_1[t] * 2
+    end
+end
+
+@defcomposite A begin
+    Component(Comp1)
+    Component(Comp2)
+
+    foo1 = Parameter(Comp1.foo)
+    foo2 = Parameter(Comp2.foo)
+
+    var_2_1 = Variable(Comp2.var_2_1)
+
+    connect(Comp2.par_2_1, Comp1.var_1_1)
+    connect(Comp2.par_2_2, Comp1.var_1_1)
+end
+
+@defcomposite B begin
+    Component(Comp3)
+    Component(Comp4)
+
+    foo3 = Parameter(Comp3.foo)
+    foo4 = Parameter(Comp4.foo)
+
+    var_3_1 = Variable(Comp3.var_3_1)
+end
+
+@defcomposite top begin
+    Component(A)
+
+    fooA1 = Parameter(A.foo1)
+    fooA2 = Parameter(A.foo2)
+
+    Component(B, first = 2010, last = 2015)
+    foo3 = Parameter(B.foo3)
+    foo4 = Parameter(B.foo4)
+
+    var_3_1 = Variable(B.var_3_1)
+
+    connect(B.par_3_1, A.var_2_1)
+    connect(B.par_4_1, B.var_3_1)
+end
+
+# We have created the following composite structure:
+#
+#          top
+#        /    \
+#       A       B
+#     /  \     /  \
+#    1    2   3    4
+
+m = Model()
+set_dimension!(m, :time, 2005:2020)
+add_comp!(m, top, nameof(top))
+
+set_param!(m, :fooA1, 1)
+set_param!(m, :fooA2, 2)
+set_param!(m, :foo3, 10)
+set_param!(m, :foo4, 20)
+set_param!(m, :par_1_1, collect(1:length(time_labels(md))))
+
+run(m)
+
+# check that first and last moved through properly
+@test m.md.first == m.md.namespace[:top].first == m.md.namespace[:top].namespace[:A].first == m.md.namespace[:top].namespace[:A].namespace[:Comp1].first== 2005
+@test m.md.last ==  m.md.namespace[:top].last == m.md.namespace[:top].namespace[:A].last ==m.md.namespace[:top].namespace[:A].namespace[:Comp1].last == 2020
+
+@test m.md.namespace[:top].namespace[:B].first == m.md.namespace[:top].namespace[:A].namespace[:Comp1].first== 2010
+@test m.md.namespace[:top].namespace[:B].last ==m.md.namespace[:top].namespace[:B].namespace[:Comp3].last == 2015
+ 
 end #module

From a5e5c904c4548f8cb29c0e106824142fd71390dc Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Tue, 9 Mar 2021 13:45:59 -0800
Subject: [PATCH 05/10] Add first and last for composites with new
 propagate_time fcn

---
 src/core/build.jl      |  3 +-
 src/core/defs.jl       | 63 ++++++++++++++++++++++++++++++++++++------
 test/test_firstlast.jl |  4 +--
 3 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/src/core/build.jl b/src/core/build.jl
index d29601707..23d64cb93 100644
--- a/src/core/build.jl
+++ b/src/core/build.jl
@@ -360,7 +360,8 @@ function _build(md::ModelDef)
     time_bounds = (firstindex(t), lastindex(t))
 
     propagate_time!(md, t) # this might not be needed, but is a final propagation to double check everything
-
+    check_all_times(md, [keys(dimension(md, :time))...])
+    
     ci = _build(md, vdict, pdict, time_bounds)
     mi = ModelInstance(ci, md)
     return mi
diff --git a/src/core/defs.jl b/src/core/defs.jl
index efafdd47f..f6e6b9268 100644
--- a/src/core/defs.jl
+++ b/src/core/defs.jl
@@ -785,23 +785,23 @@ function propagate_time!(obj::AbstractComponentDef, t::Dimension; first::Nothing
     parent_time_keys = [keys(t)...]
     if isnothing(first) && obj.first_free
         obj.first = firstindex(t)
-    elseif isnothing(first) && !obj.first_free
-        # do nothing in this case, must leave first alone it is locked
-    else
+    elseif !isnothing(first) && obj.first_free
         obj.first_free = false
         i = findfirst(isequal(first), parent_time_keys)
         isnothing(i) ? error("The given first index must exist within the parent's time dimension.") : obj.first = first
+    else
+        # nothing
     end
 
     # set last
     if isnothing(last) && obj.last_free
         obj.last = lastindex(t)
-    elseif isnothing(last) && !obj.last_free
-        # do nothing in this case, must leave last alone it is locked
-    else
+    elseif !isnothing(last) && obj.last_free
         obj.last_free = false
         i = findfirst(isequal(last), parent_time_keys)
         isnothing(i) ? error("The given last index must exist within the parent's time dimension.") : obj.last = last
+    else
+        # nothing
     end
 
     for c in compdefs(obj)      # N.B. compdefs returns empty list for leaf nodes
@@ -809,6 +809,50 @@ function propagate_time!(obj::AbstractComponentDef, t::Dimension; first::Nothing
     end
 end
 
+"""
+    propagate_time!(obj::AbstractComponentDef; first::NothingInt=nothing, last::NothingInt=nothing)
+
+Propagate just first and last through a component def tree, useful when 
+add_comp! (in the macro Composite) is called in the @defcomposite macro with a 
+first and/or last keyword argument.  If first and last keyword arguments are included 
+as integers, then the object's first_free and/or last_free flags are set to false 
+respectively, these first and last are propagated through, and they will not vary 
+freely with the model.
+"""
+function propagate_time!(obj::AbstractComponentDef; first::NothingInt=nothing, last::NothingInt=nothing)
+        
+    # note we cannot check these first/last against the time dimension yet, since 
+    # it's not set, but we do so in the build step with check_all_times
+
+    # set first
+    if !isnothing(first) && obj.first_free 
+        obj.first_free = false
+        obj.first = first
+    end
+
+    # set first
+    if !isnothing(last) && obj.last_free 
+        obj.last_free = false
+        obj.last = last
+    end
+
+    for c in compdefs(obj)      # N.B. compdefs returns empty list for leaf nodes
+        propagate_time!(c, first=first, last=last)
+    end
+end
+
+function check_all_times(obj::AbstractComponentDef, parent_time_keys::Array)
+
+    first_index = findfirst(isequal(obj.first), parent_time_keys)
+    isnothing(first_index) ? error("The first index ($(obj.first)) of component $(nameof(obj)) must exist within its model's time dimension $parent_time_keys.") : nothing
+
+    last_index = findfirst(isequal(obj.last), parent_time_keys)
+    isnothing(last_index) ? error("The last index ($(obj.last)) of component $(nameof(obj)) must exist within its model's time dimension $parent_time_keys.") : nothing
+
+    for c in compdefs(obj)      # N.B. compdefs returns empty list for leaf nodes
+        check_all_times(c, parent_time_keys[first_index:last_index])
+    end
+end
 """
     add_comp!(
         obj::AbstractCompositeComponentDef,
@@ -855,12 +899,13 @@ function add_comp!(obj::AbstractCompositeComponentDef,
     parent!(comp_def, obj)
 
     # Handle time dimension for the copy, leave the time unset for the original 
-    # component template
+    # component template - note that if dimension(obj, :time) is Nothing we can 
+    # still set first and last, which is useful for calls to Component within 
+    # the @defcomposite macro producing add_comp! calls
     if has_dim(obj, :time)
         propagate_time!(comp_def, dimension(obj, :time), first=first, last=last)
     else
-        # can't error or composites won't work
-        # error("Cannot add component to composite without first setting time dimension.")
+        propagate_time!(comp_def, first=first, last=last)
     end
 
     _add_anonymous_dims!(obj, comp_def)
diff --git a/test/test_firstlast.jl b/test/test_firstlast.jl
index 248c0eca5..ae5d3bbc5 100644
--- a/test/test_firstlast.jl
+++ b/test/test_firstlast.jl
@@ -411,7 +411,7 @@ set_param!(m, :fooA1, 1)
 set_param!(m, :fooA2, 2)
 set_param!(m, :foo3, 10)
 set_param!(m, :foo4, 20)
-set_param!(m, :par_1_1, collect(1:length(time_labels(md))))
+set_param!(m, :par_1_1, collect(1:length(time_labels(m))))
 
 run(m)
 
@@ -419,7 +419,7 @@ run(m)
 @test m.md.first == m.md.namespace[:top].first == m.md.namespace[:top].namespace[:A].first == m.md.namespace[:top].namespace[:A].namespace[:Comp1].first== 2005
 @test m.md.last ==  m.md.namespace[:top].last == m.md.namespace[:top].namespace[:A].last ==m.md.namespace[:top].namespace[:A].namespace[:Comp1].last == 2020
 
-@test m.md.namespace[:top].namespace[:B].first == m.md.namespace[:top].namespace[:A].namespace[:Comp1].first== 2010
+@test m.md.namespace[:top].namespace[:B].first == m.md.namespace[:top].namespace[:B].namespace[:Comp3].first== 2010
 @test m.md.namespace[:top].namespace[:B].last ==m.md.namespace[:top].namespace[:B].namespace[:Comp3].last == 2015
  
 end #module

From fb84596fd61a2cd2a06deab53ca2e023481772d6 Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Tue, 9 Mar 2021 17:26:02 -0800
Subject: [PATCH 06/10] Make propagate_time! modular and more generalized

---
 src/core/build.jl      |   3 +-
 src/core/defs.jl       | 113 ++++++++++++++++++++++-------------------
 src/core/dimensions.jl |   2 +-
 3 files changed, 63 insertions(+), 55 deletions(-)

diff --git a/src/core/build.jl b/src/core/build.jl
index 23d64cb93..6115646ab 100644
--- a/src/core/build.jl
+++ b/src/core/build.jl
@@ -359,8 +359,7 @@ function _build(md::ModelDef)
     t = dimension(md, :time)
     time_bounds = (firstindex(t), lastindex(t))
 
-    propagate_time!(md, t) # this might not be needed, but is a final propagation to double check everything
-    check_all_times(md, [keys(dimension(md, :time))...])
+    propagate_time!(md, t = t) # this might not be needed, but is a final propagation to double check everything
     
     ci = _build(md, vdict, pdict, time_bounds)
     mi = ModelInstance(ci, md)
diff --git a/src/core/defs.jl b/src/core/defs.jl
index f6e6b9268..8fcba7075 100644
--- a/src/core/defs.jl
+++ b/src/core/defs.jl
@@ -772,58 +772,42 @@ end
 """
     propagate_time!(obj::AbstractComponentDef, t::Dimension; first::NothingInt=nothing, last::NothingInt=nothing)
 
-Propagate a time dimension down through the comp def tree. If first and last 
-keyword arguments are included as integers, then the object's first_free and/or
-last_free flags are set to false respectively, these first and last are propagated
-through, and they will not vary freely with the model.
+Propagate a time dimension down through the comp def tree. This consists of two 
+primary functions which first push first and last through the comp def tree, and
+then push the time dimension through, which sets any leftover first and last
+attributes as well.
 """
-function propagate_time!(obj::AbstractComponentDef, t::Dimension; first::NothingInt=nothing, last::NothingInt=nothing)
-    
-    set_dimension!(obj, :time, t)
-    
-    # set first
-    parent_time_keys = [keys(t)...]
-    if isnothing(first) && obj.first_free
-        obj.first = firstindex(t)
-    elseif !isnothing(first) && obj.first_free
-        obj.first_free = false
-        i = findfirst(isequal(first), parent_time_keys)
-        isnothing(i) ? error("The given first index must exist within the parent's time dimension.") : obj.first = first
-    else
-        # nothing
-    end
+function propagate_time!(obj::AbstractComponentDef; t::Union{Dimension, Nothing}=nothing, first::NothingInt=nothing, last::NothingInt=nothing)
 
-    # set last
-    if isnothing(last) && obj.last_free
-        obj.last = lastindex(t)
-    elseif !isnothing(last) && obj.last_free
-        obj.last_free = false
-        i = findfirst(isequal(last), parent_time_keys)
-        isnothing(i) ? error("The given last index must exist within the parent's time dimension.") : obj.last = last
-    else
-        # nothing
+    # the first step is pushing through fist and last, if they are set explicitly
+    if first !== nothing || last!== nothing
+        _propagate_firstlast!(obj, first=first, last=last)
     end
 
-    for c in compdefs(obj)      # N.B. compdefs returns empty list for leaf nodes
-        propagate_time!(c, t, first=first, last=last)
+    # now propagate the actual time dimension through the components, and fill in 
+    # any missing first and lasts as defaulting to the first and last elements of the
+    # time Dimension
+    if t !== nothing
+        _propagate_time_dim!(obj, t)
+        _check_times(obj, dim_keys(obj, :time))
     end
+
+    # now run over the object and check that first and last are within the time
+    # dimension of the parent
+    
 end
 
 """
-    propagate_time!(obj::AbstractComponentDef; first::NothingInt=nothing, last::NothingInt=nothing)
-
-Propagate just first and last through a component def tree, useful when 
-add_comp! (in the macro Composite) is called in the @defcomposite macro with a 
-first and/or last keyword argument.  If first and last keyword arguments are included 
-as integers, then the object's first_free and/or last_free flags are set to false 
-respectively, these first and last are propagated through, and they will not vary 
-freely with the model.
+    _propagate_firstlast(obj::AbstractComponentDef; first::NothingInt=nothing, last::NothingInt=nothing)
+
+Propagate first and last through a component def treeIf first and last keyword 
+arguments are included as integers, then the object's first_free and/or last_free 
+flags are set to false respectively, these first and last are propagated through
+and are immutable for the future (they will not vary freely with the model's 
+time dimension).
 """
-function propagate_time!(obj::AbstractComponentDef; first::NothingInt=nothing, last::NothingInt=nothing)
+function _propagate_firstlast!(obj::AbstractComponentDef; first::NothingInt=nothing, last::NothingInt=nothing)
         
-    # note we cannot check these first/last against the time dimension yet, since 
-    # it's not set, but we do so in the build step with check_all_times
-
     # set first
     if !isnothing(first) && obj.first_free 
         obj.first_free = false
@@ -836,13 +820,40 @@ function propagate_time!(obj::AbstractComponentDef; first::NothingInt=nothing, l
         obj.last = last
     end
 
+    for c in compdefs(obj)  # N.B. compdefs returns empty list for leaf nodes     
+        _propagate_firstlast!(c, first=first, last=last)
+    end
+end
+
+"""
+    _propagate_time_dim!(obj::AbstractComponentDef, t::Dimension)
+
+Propagate a time dimension down through the comp def tree. If first and last 
+keyword arguments are not set in a given comp def they will be set to match the
+time dimension, but first_free and /or last_free flags are left as true so these
+can vary with the model's time dimension in the future. 
+"""
+function _propagate_time_dim!(obj::AbstractComponentDef, t::Dimension)
+    
+    set_dimension!(obj, :time, t)
+
+    obj.first_free && obj.first = firstindex(t)
+    obj.last_free && obj.last = lastindex(t)
+
     for c in compdefs(obj)      # N.B. compdefs returns empty list for leaf nodes
-        propagate_time!(c, first=first, last=last)
+        _propagate_time_dim!(c, t)
     end
+
 end
 
-function check_all_times(obj::AbstractComponentDef, parent_time_keys::Array)
+"""
+function _check_times(obj::AbstractComponentDef, parent_time_keys::Array)
 
+    Check that all first and last times are properly contained within a comp_def
+    `obj`'s parent time keys `parent_time_keys`.
+"""
+function _check_times(obj::AbstractComponentDef, parent_time_keys::Array)
+    
     first_index = findfirst(isequal(obj.first), parent_time_keys)
     isnothing(first_index) ? error("The first index ($(obj.first)) of component $(nameof(obj)) must exist within its model's time dimension $parent_time_keys.") : nothing
 
@@ -850,8 +861,9 @@ function check_all_times(obj::AbstractComponentDef, parent_time_keys::Array)
     isnothing(last_index) ? error("The last index ($(obj.last)) of component $(nameof(obj)) must exist within its model's time dimension $parent_time_keys.") : nothing
 
     for c in compdefs(obj)      # N.B. compdefs returns empty list for leaf nodes
-        check_all_times(c, parent_time_keys[first_index:last_index])
+        _check_times(c, parent_time_keys[first_index:last_index])
     end
+
 end
 """
     add_comp!(
@@ -899,14 +911,11 @@ function add_comp!(obj::AbstractCompositeComponentDef,
     parent!(comp_def, obj)
 
     # Handle time dimension for the copy, leave the time unset for the original 
-    # component template - note that if dimension(obj, :time) is Nothing we can 
-    # still set first and last, which is useful for calls to Component within 
+    # component template - note that if the obj does not yet have a :time dimension
+    # set we can still set first and last, which is useful for calls to Component within 
     # the @defcomposite macro producing add_comp! calls
-    if has_dim(obj, :time)
-        propagate_time!(comp_def, dimension(obj, :time), first=first, last=last)
-    else
-        propagate_time!(comp_def, first=first, last=last)
-    end
+    has_dim(obj, :time) ? t = dimension(obj, :time) : t = nothing 
+    propagate_time!(comp_def, t = first=first, last=last)
 
     _add_anonymous_dims!(obj, comp_def)
     _insert_comp!(obj, comp_def, before=before, after=after)
diff --git a/src/core/dimensions.jl b/src/core/dimensions.jl
index ca2f0dbd0..ca75c2494 100644
--- a/src/core/dimensions.jl
+++ b/src/core/dimensions.jl
@@ -106,7 +106,7 @@ function set_dimension!(ccd::AbstractCompositeComponentDef, name::Symbol, keys::
             subcomp.last_free || ccd_last < subcomp.last   && error("Top time dimension must start before or at same time as all it's subcomponents, but $(ccd_last) is before $(subcomp.last).")        
         end
 
-        propagate_time!(ccd, dim)
+        propagate_time!(ccd, t = dim)
         set_uniform!(ccd, isuniform(keys))
     end
 

From 5a818cf1d49d554f5694b546263453bb223f4122 Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Tue, 9 Mar 2021 19:58:02 -0800
Subject: [PATCH 07/10] Fix all bugs

---
 Project.toml     | 101 ++++++++++++++++++++++++-----------------------
 src/core/defs.jl |  20 +++++-----
 test/runtests.jl |   4 +-
 3 files changed, 63 insertions(+), 62 deletions(-)

diff --git a/Project.toml b/Project.toml
index 52c88b63d..a6a4500b1 100644
--- a/Project.toml
+++ b/Project.toml
@@ -3,75 +3,76 @@ uuid = "e4e893b0-ee5e-52ea-8111-44b3bdec128c"
 version = "1.1.2-DEV"
 
 [deps]
-Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
-Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
-LightGraphs = "093fc24a-ae57-5d10-9952-331d41423f4d"
-Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
 CSVFiles = "5d742f6a-9f54-50ce-8119-2520741973ca"
-Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
-MetaGraphs = "626554b9-1ddb-594c-aa3c-2596fe9399a5"
-VegaLite = "112f6efa-9a02-5b7d-90c0-432ed331239a"
-TableTraits = "3783bdb8-4a98-5b6b-af9a-565f29a5fe9c"
 Classes = "1a9c1350-211b-5766-99cd-4544d885a0d1"
-IterTools = "c8e1da08-722c-5040-9ed9-7db0dc04731e"
-Serialization = "9e88b42a-f829-5b0c-bbe9-9e923198166b"
-DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
-LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
-NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
-IteratorInterfaceExtensions = "82899510-4779-5014-852e-03e436cf321d"
-JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
-StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
+DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
 DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
-StringBuilders = "db12335b-fddc-5e1b-b0ee-42071d21ae50"
-ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca"
-GraphPlot = "a2cc645c-3eea-5389-862e-a155d0052231"
-MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
-FileIO = "5789e2e9-d7fb-5bc7-8068-2c6fae9b9549"
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
-GlobalSensitivityAnalysis = "1b10255b-6da3-57ce-9089-d24e8517b87e"
-DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
+DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
+Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
 Electron = "a1bb12fb-d4d1-54b4-b10a-ee7951ef7ad3"
-Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
+FileIO = "5789e2e9-d7fb-5bc7-8068-2c6fae9b9549"
 FilePaths = "8fc22ac5-c921-52a6-82fd-178b2807b824"
-
-[extras]
+GlobalSensitivityAnalysis = "1b10255b-6da3-57ce-9089-d24e8517b87e"
+GraphPlot = "a2cc645c-3eea-5389-862e-a155d0052231"
+IterTools = "c8e1da08-722c-5040-9ed9-7db0dc04731e"
+IteratorInterfaceExtensions = "82899510-4779-5014-852e-03e436cf321d"
+JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
+LightGraphs = "093fc24a-ae57-5d10-9952-331d41423f4d"
+LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
+Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
+MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
+MetaGraphs = "626554b9-1ddb-594c-aa3c-2596fe9399a5"
+MimiFUND = "b3ba11de-429f-11e9-29f7-cb478ab96e7c"
+NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
 Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
-Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
-Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
-Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
-ZipFile = "a5390f91-8eb1-5f08-bee0-b1d1ffed6cea"
-Query = "1a8c2f83-1ff3-5112-b086-8aa67b057ba1"
-DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
-ExcelReaders = "c04bee98-12a5-510c-87df-2a230cb6e075"
-NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
-MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
-ExcelFiles = "89b67f3b-d1aa-5f6f-9ca4-282e8d98620d"
+Serialization = "9e88b42a-f829-5b0c-bbe9-9e923198166b"
+Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
+StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
+StringBuilders = "db12335b-fddc-5e1b-b0ee-42071d21ae50"
+TableTraits = "3783bdb8-4a98-5b6b-af9a-565f29a5fe9c"
+VegaLite = "112f6efa-9a02-5b7d-90c0-432ed331239a"
 
 [compat]
-LightGraphs = "1.3"
-Distributions = "0.21, 0.22, 0.23, 0.24"
 CSVFiles = "0.16, 1.0"
-MetaGraphs = "0.6"
-VegaLite = "1, 2"
-TableTraits = "0.4.1, 1"
 Classes = "1.2"
+DataFrames = "0.19.1, 0.20, 0.21, 0.22"
+DataStructures = "0.17, 0.18"
+Distributions = "0.21, 0.22, 0.23, 0.24"
 Electron = "3.1"
+FileIO = "1"
+FilePaths = "0.8"
+GlobalSensitivityAnalysis = "1.0"
+GraphPlot = "0.3, 0.4"
 IterTools = "1.3"
-NamedArrays = "0.9"
 IteratorInterfaceExtensions = "0.1.1, 1"
 JSON = "0.21"
+LightGraphs = "1.3"
+MacroTools = "0.5"
+MetaGraphs = "0.6"
+NamedArrays = "0.9"
+ProgressMeter = "1.2"
 StatsBase = "0.32, 0.33"
-DataStructures = "0.17, 0.18"
 StringBuilders = "0.2"
-ProgressMeter = "1.2"
-GraphPlot = "0.3, 0.4"
-MacroTools = "0.5"
-FileIO = "1"
+TableTraits = "0.4.1, 1"
+VegaLite = "1, 2"
 julia = "1.4"
-GlobalSensitivityAnalysis = "1.0"
-DataFrames = "0.19.1, 0.20, 0.21, 0.22"
-FilePaths = "0.8"
+
+[extras]
+DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
+Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
+ExcelFiles = "89b67f3b-d1aa-5f6f-9ca4-282e8d98620d"
+ExcelReaders = "c04bee98-12a5-510c-87df-2a230cb6e075"
+MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
+NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
+Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
+Query = "1a8c2f83-1ff3-5112-b086-8aa67b057ba1"
+Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
+Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+ZipFile = "a5390f91-8eb1-5f08-bee0-b1d1ffed6cea"
 
 [targets]
 test = ["Pkg", "Statistics", "NamedArrays", "ExcelFiles", "ExcelReaders", "DelimitedFiles", "Documenter", "Random", "Test", "ZipFile", "MacroTools", "Query"]
diff --git a/src/core/defs.jl b/src/core/defs.jl
index 8fcba7075..138513b04 100644
--- a/src/core/defs.jl
+++ b/src/core/defs.jl
@@ -784,17 +784,17 @@ function propagate_time!(obj::AbstractComponentDef; t::Union{Dimension, Nothing}
         _propagate_firstlast!(obj, first=first, last=last)
     end
 
-    # now propagate the actual time dimension through the components, and fill in 
-    # any missing first and lasts as defaulting to the first and last elements of the
-    # time Dimension
     if t !== nothing
+        # propagate the actual time dimension through the components, and fill in 
+        # any missing first and lasts as defaulting to the first and last elements of the
+        # time Dimension
         _propagate_time_dim!(obj, t)
-        _check_times(obj, dim_keys(obj, :time))
+
+        # run over the object and check that first and last are within the time
+        # dimension of the parent
+        _check_times(obj, [keys(t)...])
     end
 
-    # now run over the object and check that first and last are within the time
-    # dimension of the parent
-    
 end
 
 """
@@ -837,8 +837,8 @@ function _propagate_time_dim!(obj::AbstractComponentDef, t::Dimension)
     
     set_dimension!(obj, :time, t)
 
-    obj.first_free && obj.first = firstindex(t)
-    obj.last_free && obj.last = lastindex(t)
+    obj.first_free ? obj.first = firstindex(t) : nothing
+    obj.last_free ? obj.last = lastindex(t) : nothing
 
     for c in compdefs(obj)      # N.B. compdefs returns empty list for leaf nodes
         _propagate_time_dim!(c, t)
@@ -915,7 +915,7 @@ function add_comp!(obj::AbstractCompositeComponentDef,
     # set we can still set first and last, which is useful for calls to Component within 
     # the @defcomposite macro producing add_comp! calls
     has_dim(obj, :time) ? t = dimension(obj, :time) : t = nothing 
-    propagate_time!(comp_def, t = first=first, last=last)
+    propagate_time!(comp_def, t = t, first=first, last=last)
 
     _add_anonymous_dims!(obj, comp_def)
     _insert_comp!(obj, comp_def, before=before, after=after)
diff --git a/test/runtests.jl b/test/runtests.jl
index 00f10e7d0..c6711fc6a 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -53,8 +53,8 @@ Electron.prep_test_env()
     @info("test_replace_comp.jl")
     @time include("test_replace_comp.jl")
 
-    @info("test_tools.jl")
-    @time include("test_tools.jl")
+    # @info("test_tools.jl")
+    # @time include("test_tools.jl")
 
     @info("test_parameter_labels.jl")
     @time include("test_parameter_labels.jl")

From 1589249351fff12505ee8d44647c7776a4313cd3 Mon Sep 17 00:00:00 2001
From: Lisa Rennels <31779240+lrennels@users.noreply.github.com>
Date: Tue, 9 Mar 2021 21:11:28 -0800
Subject: [PATCH 08/10] reenable test_tools.jl

---
 test/runtests.jl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/runtests.jl b/test/runtests.jl
index c6711fc6a..00f10e7d0 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -53,8 +53,8 @@ Electron.prep_test_env()
     @info("test_replace_comp.jl")
     @time include("test_replace_comp.jl")
 
-    # @info("test_tools.jl")
-    # @time include("test_tools.jl")
+    @info("test_tools.jl")
+    @time include("test_tools.jl")
 
     @info("test_parameter_labels.jl")
     @time include("test_parameter_labels.jl")

From 98e32c11840aeb4d4f8966bbbbbfe9ed74bf8b3d Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Tue, 9 Mar 2021 21:12:32 -0800
Subject: [PATCH 09/10] Revert project.toml changes

---
 Project.toml | 101 +++++++++++++++++++++++++--------------------------
 1 file changed, 50 insertions(+), 51 deletions(-)

diff --git a/Project.toml b/Project.toml
index a6a4500b1..52c88b63d 100644
--- a/Project.toml
+++ b/Project.toml
@@ -3,76 +3,75 @@ uuid = "e4e893b0-ee5e-52ea-8111-44b3bdec128c"
 version = "1.1.2-DEV"
 
 [deps]
+Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
+Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
+LightGraphs = "093fc24a-ae57-5d10-9952-331d41423f4d"
+Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
 CSVFiles = "5d742f6a-9f54-50ce-8119-2520741973ca"
+Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+MetaGraphs = "626554b9-1ddb-594c-aa3c-2596fe9399a5"
+VegaLite = "112f6efa-9a02-5b7d-90c0-432ed331239a"
+TableTraits = "3783bdb8-4a98-5b6b-af9a-565f29a5fe9c"
 Classes = "1a9c1350-211b-5766-99cd-4544d885a0d1"
-DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
-DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
-Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
-DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
-Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
-Electron = "a1bb12fb-d4d1-54b4-b10a-ee7951ef7ad3"
-FileIO = "5789e2e9-d7fb-5bc7-8068-2c6fae9b9549"
-FilePaths = "8fc22ac5-c921-52a6-82fd-178b2807b824"
-GlobalSensitivityAnalysis = "1b10255b-6da3-57ce-9089-d24e8517b87e"
-GraphPlot = "a2cc645c-3eea-5389-862e-a155d0052231"
 IterTools = "c8e1da08-722c-5040-9ed9-7db0dc04731e"
+Serialization = "9e88b42a-f829-5b0c-bbe9-9e923198166b"
+DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
+LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
+NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
 IteratorInterfaceExtensions = "82899510-4779-5014-852e-03e436cf321d"
 JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
-LightGraphs = "093fc24a-ae57-5d10-9952-331d41423f4d"
-LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
-Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
+StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
+DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
+StringBuilders = "db12335b-fddc-5e1b-b0ee-42071d21ae50"
+ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca"
+GraphPlot = "a2cc645c-3eea-5389-862e-a155d0052231"
 MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
-MetaGraphs = "626554b9-1ddb-594c-aa3c-2596fe9399a5"
-MimiFUND = "b3ba11de-429f-11e9-29f7-cb478ab96e7c"
-NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
+FileIO = "5789e2e9-d7fb-5bc7-8068-2c6fae9b9549"
+Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
+GlobalSensitivityAnalysis = "1b10255b-6da3-57ce-9089-d24e8517b87e"
+DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
+Electron = "a1bb12fb-d4d1-54b4-b10a-ee7951ef7ad3"
+Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
+FilePaths = "8fc22ac5-c921-52a6-82fd-178b2807b824"
+
+[extras]
 Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
-ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca"
-Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
-Serialization = "9e88b42a-f829-5b0c-bbe9-9e923198166b"
 Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
-StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
-StringBuilders = "db12335b-fddc-5e1b-b0ee-42071d21ae50"
-TableTraits = "3783bdb8-4a98-5b6b-af9a-565f29a5fe9c"
-VegaLite = "112f6efa-9a02-5b7d-90c0-432ed331239a"
+Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
+Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+ZipFile = "a5390f91-8eb1-5f08-bee0-b1d1ffed6cea"
+Query = "1a8c2f83-1ff3-5112-b086-8aa67b057ba1"
+DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
+ExcelReaders = "c04bee98-12a5-510c-87df-2a230cb6e075"
+NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
+MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
+ExcelFiles = "89b67f3b-d1aa-5f6f-9ca4-282e8d98620d"
 
 [compat]
+LightGraphs = "1.3"
+Distributions = "0.21, 0.22, 0.23, 0.24"
 CSVFiles = "0.16, 1.0"
+MetaGraphs = "0.6"
+VegaLite = "1, 2"
+TableTraits = "0.4.1, 1"
 Classes = "1.2"
-DataFrames = "0.19.1, 0.20, 0.21, 0.22"
-DataStructures = "0.17, 0.18"
-Distributions = "0.21, 0.22, 0.23, 0.24"
 Electron = "3.1"
-FileIO = "1"
-FilePaths = "0.8"
-GlobalSensitivityAnalysis = "1.0"
-GraphPlot = "0.3, 0.4"
 IterTools = "1.3"
+NamedArrays = "0.9"
 IteratorInterfaceExtensions = "0.1.1, 1"
 JSON = "0.21"
-LightGraphs = "1.3"
-MacroTools = "0.5"
-MetaGraphs = "0.6"
-NamedArrays = "0.9"
-ProgressMeter = "1.2"
 StatsBase = "0.32, 0.33"
+DataStructures = "0.17, 0.18"
 StringBuilders = "0.2"
-TableTraits = "0.4.1, 1"
-VegaLite = "1, 2"
+ProgressMeter = "1.2"
+GraphPlot = "0.3, 0.4"
+MacroTools = "0.5"
+FileIO = "1"
 julia = "1.4"
-
-[extras]
-DelimitedFiles = "8bb1440f-4735-579b-a4ab-409b98df4dab"
-Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
-ExcelFiles = "89b67f3b-d1aa-5f6f-9ca4-282e8d98620d"
-ExcelReaders = "c04bee98-12a5-510c-87df-2a230cb6e075"
-MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
-NamedArrays = "86f7a689-2022-50b4-a561-43c23ac3c673"
-Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
-Query = "1a8c2f83-1ff3-5112-b086-8aa67b057ba1"
-Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
-Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
-Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
-ZipFile = "a5390f91-8eb1-5f08-bee0-b1d1ffed6cea"
+GlobalSensitivityAnalysis = "1.0"
+DataFrames = "0.19.1, 0.20, 0.21, 0.22"
+FilePaths = "0.8"
 
 [targets]
 test = ["Pkg", "Statistics", "NamedArrays", "ExcelFiles", "ExcelReaders", "DelimitedFiles", "Documenter", "Random", "Test", "ZipFile", "MacroTools", "Query"]

From c92ea15f318f432fa61538983e94cd9ad3584c38 Mon Sep 17 00:00:00 2001
From: lrennels <lrennels@berkeley.edu>
Date: Thu, 11 Mar 2021 12:31:41 -0800
Subject: [PATCH 10/10] Make first and last specific to connector comp names

---
 src/core/connections.jl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/core/connections.jl b/src/core/connections.jl
index f9ee730f7..b08629ef4 100644
--- a/src/core/connections.jl
+++ b/src/core/connections.jl
@@ -642,8 +642,8 @@ function add_connector_comps!(obj::AbstractCompositeComponentDef)
             param_first = first_period(obj, src_comp_def)
             conn.backup_offset !== nothing ? param_first = param_first + conn.backup_offset : nothing
 
-            set_param!(obj, conn_comp_name, :first, param_first)
-            set_param!(obj, conn_comp_name, :last, param_last)
+            set_param!(obj, conn_comp_name, :first, Symbol(conn_comp_name, "_", :first), param_first)
+            set_param!(obj, conn_comp_name, :last, Symbol(conn_comp_name, "_", :last), param_last)
         end
     end