-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add in-place conversion to basic data format #915
Conversation
Shall we add a test for |
I can do that. I'm currently in a plane, will push in a couple days |
After that I'll be good to merge. Feel free to add your name in the list of the readme if you like. |
@test data == data_ # input data not modified | ||
@test data_basic !== data # we created a new dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These ones check that we did not modify the input data, and that we returned a new Dict
object
test/data-basic.jl
Outdated
|
||
# Test #2: `make_basic_network!` is destructive and works in-place | ||
data_basic = PowerModels.make_basic_network!(data) | ||
@test data_copy === data # in-place modification |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I only check that the returned dictionary is exactly the one that was passed as input.
Note the use of triple equal ===
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just fixed a typo: data_copy
should have been data_basic
here
# Test #3: double-check the output of `make_basic_network!` | ||
# This may be redundant with the above tests, unless `make_basic_network` does not | ||
# call the in-place version under the hood | ||
@test length(data_basic["bus"]) == 4 | ||
@test length(data_basic["load"]) == 2 | ||
@test length(data_basic["shunt"]) == 2 | ||
@test length(data_basic["gen"]) == 1 | ||
@test length(data_basic["branch"]) == 2 | ||
@test length(data_basic["dcline"]) == 0 | ||
@test length(data_basic["switch"]) == 0 | ||
@test length(data_basic["storage"]) == 0 | ||
|
||
result = solve_opf(data_basic, ACPPowerModel, nlp_solver) | ||
@test isapprox(result["objective"], 1036.52; atol=1e0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those extra tests are a safety net in case someone later decides to not call make_basic_network!
inside of the non-destructive make_basic_network
* Improve performance of calc_connected_components (#914) --------- Co-authored-by: mtanneau3 <[email protected]> * Add in-place conversion to basic data format (#915) * Add in-place conversion to basic data format * Add unit test for in-place make_basic_network --------- Co-authored-by: mtanneau3 <[email protected]> * No deepcopy when renumbering components (#916) * No deepcopy when renumbering components * In-place _renumber_components and unit tests * Fix typo --------- Co-authored-by: mtanneau3 <[email protected]> Co-authored-by: Carleton Coffrin <[email protected]> * add notes to changelog and prep for release * Update psse parser for 3winding transformers (#917) * fix psse parser 3 winding tranformers * update readme * add note to changelog and readme * fix quote counting check in psse parser --------- Co-authored-by: Mathieu Tanneau <[email protected]> Co-authored-by: mtanneau3 <[email protected]> Co-authored-by: Rahmat Heidari <[email protected]>
Motivated by #913
This PR introduces an in-place
make_basic_data!
, which has the same functionality but does not create adeepcopy
.The goal is to avoid the (time and memory) cost of deepcopy-ing the initial data dictionary when the user does not want to keep it.
A typical use-case would be workflows that look like
data = make_basic_network(parse_file(...))
.This call with load the raw data dictionary with
parse_file
, then deep-copy it, convert the copy to basic format, and discard the original raw data.Timings of
make_basic_network
after the changes: times are improved, and memory consumption is reduced.Note: those are just the ouptut of
@time
; I did runGC.gc()
before to reduce garbage collection, but it's not a full-fledge@benchmark
1354_pegase
2869_pegase
9241_pegase
13659_pegase
and corresponding profile of
make_basic_network!(data)
wheredata = pglib("9241_pegase")
:Note that the above profile does not include the improvements in #914. The main (and expected) outcome is that the time spent deep-copying at the beginning is gone.