Read export metadata file for round trip with PowerFlows.jl's PSS/E exporter #1273
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (17.10%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1273 +/- ##
==========================================
- Coverage 84.35% 83.74% -0.62%
==========================================
Files 183 184 +1
Lines 8451 8519 +68
==========================================
+ Hits 7129 7134 +5
- Misses 1322 1385 +63
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| function remap_bus_numbers!(sys::System, bus_number_mapping) | ||
| for bus in collect(get_components(Bus, sys)) | ||
| set_number!(bus, parse(Int, bus_number_mapping[get_number(bus)])) | ||
| end |
There was a problem hiding this comment.
sys.bus_numbers is now out-of-date. You also need to guarantee unique numbers.
There was a problem hiding this comment.
The bus_number_mapping is a reversed dictionary so that guarantees uniqueness. I can modify sys.bus_numbers, but isn't it a bug for public interface to be able to put the system into an inconsistent state?
There was a problem hiding this comment.
We could set exclude_setter = true in power_system_structs.json and then add a function for set_bus!(::System, ::Bus), similar to set_name!.
There was a problem hiding this comment.
I think this solution is ok for now and we can revise the bus renumbering in PSY 6. I think that when we re-design the capability to read/write to the DB this will come back
There was a problem hiding this comment.
I'll fix it as a one-off here for now, principled solution tracked at #1274
| end | ||
|
|
||
| function remap_bus_numbers!(sys::System, bus_number_mapping) | ||
| for bus in collect(get_components(Bus, sys)) |
There was a problem hiding this comment.
why are we calling collect here?
There was a problem hiding this comment.
-> #1275 |
Here, I implement logic such that when
System("my_filename.raw")is called, PowerSystems will check for amy_filename_export_metadata.jsonfile written by the PowerFlows.jl PSS/E exporter in the same directory and, if it is found, use its contents to undo the system alterations PowerFlows had to make to write to the PSS/E format. The goal is for systems loaded this way to look as similar to the original system that was serialized as possible. The user can opt out of this feature by passingtry_reimport = false. There is also interface to manually pass a metadata dictionary to perform this same remapping.In the process of implementation, I also added a
area_name_formatterSystem constructor kwarg along the same lines as #1160.No unit tests are currently included. Up until now, this functionality was implemented in PowerFlows.jl, and there are fairly detailed but not perfectly comprehensive tests of this functionality due to the testing of the round trip fidelity over there. Looking for reviewer input on whether that is sufficient to merge this now or whether dedicated unit tests in PowerSystems are required first.
Exporter is implemented in NREL-Sienna/PowerFlows.jl#53 and that PR now depends on this one.
@daniel-thom don't feel obligated to fully review, just tagging you in case you have opinions on how I'm messing with the System constructor.