feat: add REISEjlMATReader to interpret input.mat from REISE.jl#150
feat: add REISEjlMATReader to interpret input.mat from REISE.jl#150danielolsen merged 5 commits intodevelopfrom
Conversation
|
Looks good. I like the different tests you carried out to make sure that outputs were kept the same for the refactored MATReader and new REISEMATReader and that REISEjlMATREader was consistent with REISEMATReader on the same scenario. |
|
I wanted to be extra sure I didn't accidentally break anything. It may be worthwhile to adapt some of this test code into an |
|
What do you think of the naming of the classes? It is outside the scope of this PR but I feel like we can be more explicit. For instance: |
dae321c to
444abe7
Compare
444abe7 to
54b7110
Compare
|
@rouille I am agnostic to the naming, as long as the code works ;) I think it would be fine to address in this PR, if we decide on new names before we are ready to merge the functional code. Note for reviewers: since the fix in #151, I have not yet repeated the integration tests performed in the original. The new Grid equality tests should be useful here. |
|
What do you think @BainanXia of the current naming. Is it worth changing the name of the objects? |
|
@rouille I like the new names you proposed. One of the reason is that the naming should focus on the high-level functionality of the code instead of including more details of the properties, which will be easier to scale in the future. For example, if we have other engines (or same engine) dealing with other type of input files (not .mat anymore), we can still call it |
|
@danielolsen, do you mind changing the names. I guess we then also need to rename mat_reader.py as scenario_grid.py. Sorry for the extra work. |
|
No problem, filenames have been changed and all tests still pass. |
|
Let's check again that we didn't corrupt the Grid objects as instantiated by REISEMATReader/FromREISE are not corrupted. With With We can also confirm that the scenarios match between FromREISE and FromREISEjl: Wait... I added re-indexing of the storage rows if they are present, and told grid equality to ignore |
|
Thanks for the extra tests and debugging my code! |
BainanXia
left a comment
There was a problem hiding this comment.
This is solid! @danielolsen did the test that I was about to do. Thanks.
44102f9 to
b692479
Compare
Purpose
To be able to interpret
input.matfiles generated by REISE.jl, so that we can recreate a Grid based on files saved on the server. This relies on REISE.jlsavingconverting all the extra data we need, see Breakthrough-Energy/REISE.jl#37.What is the code doing
In grid.py: we import the new REISEjlMATReader and instantiate it when we pass an
engineof'REISE.jl'.In
mat_reader.py:REISEMATReader._build_network()toMATReader._read_network(), because it can be reused.add_information_to_modelfunction, and into a new functionreindex_model, so thatadd_information_to_modelonly has 'adding columns to dataframes' code._build_network()methods in bothREISEMATReaderandREISEjlMATReaderextremely short: they both call_read_network(fromMATReader), REISEMATReader callsreindex_model, and then they both calladd_information_to_model.Documentation that everything works as intended
Unit tests all pass.
For an integration test, we will start with Scenario 419 (Western with DC lines). First, we need to run REISE.jl
with theto generate thesave_extra_grid_databranch checked out to save the extra info toinput.matfile. Then, we need to locally run the conversion from a v7.3 matfile to a v7 matfile (see Breakthrough-Energy/REISE.jl#37 which has code forboth of theseconversion). We save the new file asinput2.mat.First, we want to make sure that we have not changed anything from the old REISEMATReader object, so with
developon powersimdata checked out:Now let's check out the
reisejlmatreaderbranch for powersimdata, re-generate the Grid from theinput.matfile, and confirm that they are equal:Great! So our refactor has not changed anything in the Grid objects generated by the
REISEMATReaderobjects. But what about the Grid objects generated by the newREISEjlMATReaderobjects?Great! The Grids created by reading the
input.matfiles are identical.Repeating for Scenario 160, which has storage:
So we can confirm that the refactor of REISEMATReader didn't mess with anything to do with storage either. Confirming that the new REISEjlMATReader works is a bit more involved, because the
case.matfile for Scenario 160 on the server was created before we started saving all our extra info in there. @rouille did the reprocessing in the160_grid.matfile on the server, so let's use that file to create thecase.matfile that we would expect to see created today. In MATLAB:Now we can use this
case.matfile to run REISE.jl, which will produce aninput.matfile, which can be converted from v7.3 to v7. Finally, we can load this file in and confirm that the created Grid objects are the same:Again, REISE.jl has saved all required data, and REISEjlMATReader knows how to interpret it, so that it matches exactly with what was created and read by REISE!
Time to review
One hour. The code itself is fairly simple, but the documentation steps are fairly involved.