Closed ccoffrin closed 4 years ago
merge_solution
does not combine the data dictionary in the two solutions. (this is what I was running into when I wanted to check energy not served)
I believe the reported solution here will not have a data dictionary in a multi-network format with all required time-periods.
I think there is a confusion about the specification of the result dict. The value in "data" is usually just some metadata about the instance. For example, see here,
https://github.com/lanl-ansi/PowerModels.jl/blob/v0.14.3/src/core/solution.jl#L39
If you would like to return the mnnetwork data it should be returned as another field in the result dict. However, that approach would also not be consistent with the standard `run*methods. In general they should be single-network-data-in/single-network-solution-out or multi-network-data-in/multi-network-solution-out. So that calling
update_data!` with the solution is well defined.
Also I am considering removing the "data" field from the results becouse I don't think it is very useful at this point.
That would be my confusion...that "data" should be a copy of the input data.
Then it will be fine
Ok great. I will merge this once travis passes.
Merging #36 into master will decrease coverage by
0.01%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #36 +/- ##
==========================================
- Coverage 92.82% 92.81% -0.02%
==========================================
Files 22 22
Lines 1450 1447 -3
==========================================
- Hits 1346 1343 -3
Misses 104 104
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 022413c...43c08c1. Read the comment docs.
@noahrhodes, as far as I can tell this is a correct implementation of the results file, but I know you changed this recently. What am I missing?