Simulation update funcs #478
Conversation
- Eliminated sim.dist_rvs - Changed uses of sim.dist_rvs to call values(sim.rvdict)
|
These new fns are not used in the code base. I though we might merge them to see if they meet our needs. |
|
These functions look great to me! I'm looking at the Travis build though, should we be concerned that something is erroring when trying to do an LHS sample? |
| using Mimi: reset_compdefs, modelinstance, compinstance, | ||
| get_var_value, OUTER, INNER, ReshapedDistribution | ||
|
|
||
| reset_compdefs() |
There was a problem hiding this comment.
Why is this removed? (I'm not sure I understand why it was there in the first place, but just wondering what's different now)
There was a problem hiding this comment.
This follows from the removal of the global list of component defs. Nothing to reset!
src/mcs/lhs.jl
Outdated
| rvdict = sim.rvdict | ||
| num_rvs = length(rvdict) | ||
| rvlist = sim.dist_rvs | ||
| rvlist = collect(values(sim.rvdict)) |
There was a problem hiding this comment.
I just added this extra collect because the lhs function that gets called below expects a vector of RVs, not an iterator. All of the other places where you replace sim.dist_rvs with values(sim.rvdict) work though because they only get used in for loops
|
I do think we should get back into the grove of not merging things that break CI. Can we figure out what is happening there before we merge? |
|
@davidanthoff yes there is an actual error, I fixed one of them but there's still another so we definitely should not merge this yet! |
|
I just added another spot where we need to use |
|
I just posted an update to this branch that passes all Mimi tests. |
Codecov Report
@@ Coverage Diff @@
## master #478 +/- ##
========================================
- Coverage 83.26% 82% -1.26%
========================================
Files 26 26
Lines 1870 1901 +31
========================================
+ Hits 1557 1559 +2
- Misses 313 342 +29
Continue to review full report at Codecov.
|
|
Ready to merge then? @rjplevin do you want to merge, you probably know best whether this is ready? |
|
I'm testing against all the models... I'll merge it when everything passes! |
|
All Mimi and model tests are now passing. |
Simulationinstancesim.dist_rvssim.dist_rvsto callvalues(sim.rvdict)See also my comments in #474