WIP Add payload retrieval function for SimulationInstance

[corakingdon]

I think this method should be added in order to more easily access a payload object during a post trial function. Without it, the user has to do Mimi.payload(sim_inst.sim_def).

I'm wondering though if it would make more sense for a copy of the payload object to be created as a field in SimulationInstance, instead of having it be stored in the copy of the simulation definition. Thoughts? @lrennels @davidanthoff

[lrennels]

Ah interesting, yes I guess that gets to the heart of whether something like a post trial function would be considered part of the more general SimulationDef, or the specific SimulationInstance. I think my gut reaction is that since the post_trial_func is passed along with SimulationDef to run, it should be part of the SimulationInstance, because I think at that point the idea would be that as much as possible the definition is immutable?

[davidanthoff]

I have to admit I'm not familiar enough with the design right now to have an opinion. I think either you can decide, or we discuss it on Fri?

[corakingdon]

I'm pretty sure it would make more sense for there to be a payload field in the SimulationInstance. At least in all the cases I have of using the payload, it is something that gets modified (i.e. for storage of values).

I'll try to make those changes to Mimi on this branch and we can look at it/discuss on Friday.

[corakingdon]

I just added the very small changes. I realized that creating an actual field in the SimulationInstance (like I just added) might not be necessary, since the deepcopy of the sim_def already has a copy of the payload. But if we want to treat the deepcopy of the sim_def as immutable, then it makes sense to make a separate copy of the payload in the SimluationInstance.

Flow:

• user sets a payload object on a SimulationDefinition with Mimi.set_payload!(simdef, payload)
• at run time, a SimulationInstance is created, which holds a deepcopy of the sim_def, and an additional copy of the payload object.
• during pre/post trial or scenario functions, the user can modify the copy of the payload in the SimulationInstance
• after the simulation is run, the user can access the modified payload in the SimulationInstance with Mimi.payload(sim_inst)

Example:

using Mimi
  
@defcomp c begin
end

m = Model()
set_dimension!(m, :time, 1:2)
add_comp!(m, c)

function post_trial(sim_inst::SimulationInstance, trialnum::Int, ntimesteps::Int, tup::Union{Nothing, Tuple})
    data = Mimi.payload(sim_inst)
    data[trialnum] = trialnum
end

sim_def = @defsim begin
end

trials = 10

original_payload = zeros(trials)
Mimi.set_payload!(sim_def, original_payload)

sim_inst = run(sim_def, m, trials, post_trial_func = post_trial)

Mimi.payload(sim_def) == original_payload   # in the original defintion, it's still zeros
Mimi.payload(sim_inst) == collect(1:trials) # in the instance, it's now 1:10
Mimi.payload(sim_inst.sim_def) == original_payload  # the definition stored in the instance still holds the unmodified payload object

[lrennels]

Sounds good to me, I think keeping the central tenets of an immutable definition and a mutable instance is great and then we can play off of that. I think a test is failing with explore(sim::SimulationInstance), @ckingdon95 let me know If you need help digging that I'd happily help!

[lrennels]

@ckingdon95 since we can't merge until all tests pass, can you merge in the master branch which will fix the failure of the dependency testing?

[codecov-io]

Codecov Report

Merging #543 into master will increase coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master     #543     +/-   ##
=========================================
+ Coverage    83.7%   83.81%   +0.1%     
=========================================
  Files          27       27             
  Lines        2123     2125      +2     
=========================================
+ Hits         1777     1781      +4     
+ Misses        346      344      -2

Impacted Files	Coverage Δ
src/mcs/mcs_types.jl	80.85% <100%> (+5.29%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24b7ccc...2950224. Read the comment docs.