-
Notifications
You must be signed in to change notification settings - Fork 149
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
Update psse parser for 3winding transformers #917
Conversation
src/io/psse.jl
Outdated
if transformer["CW"] != 1 # NOT "for off-nominal turns ratio in pu of winding bus base voltage" | ||
sub_data["tap"] /= _get_bus_value(bus_id, "base_kv", pm_data) | ||
if transformer["CW"] == 3 # "for off-nominal turns ratio in pu of nominal winding voltage, NOMV1, NOMV2 and NOMV3." | ||
sub_data["tap"] *= transformer["NOMV$m"] | ||
if transformer["CW"] == 2 | ||
sub_data["tap"] /= _get_bus_value(bus_id, "base_kv", pm_data) | ||
elseif transformer["CW"] == 3 # "for off-nominal turns ratio in pu of nominal winding voltage, NOMV1, NOMV2 and NOMV3." | ||
if !iszero(transformer["NOMV$m"]) | ||
sub_data["tap"] *= transformer["NOMV$m"] / _get_bus_value(bus_id, "base_kv", pm_data) | ||
end | ||
end | ||
end |
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.
Could you elaborate about what the correct semantics of this should be?
The way the code is organized seems risky to me. For example, what happens if transformer["CW"] == 3
and iszero(transformer["NOMV$m"]) == true
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.
iszero(transformer["NOMV$m"]) == true
means that the transformer winding m
nominal voltage is the same as the bus base voltage it is connected to, i.e. _get_bus_value(bus_id, "base_kv", pm_data)
. So in the case of transformer["CW"] == 3
, the tap value won't change. For when transformer["CW"] == 2
, the winding voltage is in kv and does not need to account for transformer["NOMV$m"]
.
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.
Given the complexity of this sepecific section I suggest we re-write the code int he form,
if transformer["CW"] == 1
...
elseif transformer["CW"] == 2
...
elseif transformer["CW"] == 3
...
else
error - unsupported value for "CW" on transformer ....
end
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.
Recommendation implemented. :)
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.
Ok, I think this is very close to ready. I do have one last question about float equality testing to resolve.
src/io/psse.jl
Outdated
@@ -434,8 +434,13 @@ function _psse2pm_transformer!(pm_data::Dict, pti_data::Dict, import_all::Bool) | |||
else | |||
br_r, br_x = transformer["R1-2"], transformer["X1-2"] | |||
end | |||
br_r *= (transformer["NOMV1"]^2 / _get_bus_value(transformer["I"], "base_kv", pm_data)^2) * (pm_data["baseMVA"] / transformer["SBASE1-2"]) | |||
br_x *= (transformer["NOMV1"]^2 / _get_bus_value(transformer["I"], "base_kv", pm_data)^2) * (pm_data["baseMVA"] / transformer["SBASE1-2"]) | |||
if transformer["NOMV1"] == 0.0 |
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.
Instead of x == 0.0
I am wondering if isapprox(x, 0.0)
would be more robust.
@odow, can you comment the the best practice in Julia for float equality tests?
src/io/psse.jl
Outdated
@@ -448,8 +453,13 @@ function _psse2pm_transformer!(pm_data::Dict, pti_data::Dict, import_all::Bool) | |||
br_r *= (transformer["WINDV2"]/_get_bus_value(transformer["J"], "base_kv", pm_data))^2 | |||
br_x *= (transformer["WINDV2"]/_get_bus_value(transformer["J"], "base_kv", pm_data))^2 | |||
else # "for off-nominal turns ratio in pu of nominal winding voltage, NOMV1, NOMV2 and NOMV3." | |||
br_r *= (transformer["WINDV2"]*(transformer["NOMV2"]/_get_bus_value(transformer["J"], "base_kv", pm_data)))^2 | |||
br_x *= (transformer["WINDV2"]*(transformer["NOMV2"]/_get_bus_value(transformer["J"], "base_kv", pm_data)))^2 | |||
if transformer["NOMV2"] == 0.0 |
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.
Same note as above.
src/io/psse.jl
Outdated
br_r12 *= (transformer["NOMV1"] / _get_bus_value(bus_id1, "base_kv", pm_data))^2 * (pm_data["baseMVA"] / transformer["SBASE1-2"]) | ||
br_r23 *= (transformer["NOMV2"] / _get_bus_value(bus_id2, "base_kv", pm_data))^2 * (pm_data["baseMVA"] / transformer["SBASE2-3"]) | ||
br_r31 *= (transformer["NOMV3"] / _get_bus_value(bus_id3, "base_kv", pm_data))^2 * (pm_data["baseMVA"] / transformer["SBASE3-1"]) | ||
if transformer["NOMV1"] == 0.0 |
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.
same note as above.
src/io/psse.jl
Outdated
br_x12 *= (transformer["NOMV1"] / _get_bus_value(bus_id1, "base_kv", pm_data))^2 * (pm_data["baseMVA"] / transformer["SBASE1-2"]) | ||
br_x23 *= (transformer["NOMV2"] / _get_bus_value(bus_id2, "base_kv", pm_data))^2 * (pm_data["baseMVA"] / transformer["SBASE2-3"]) | ||
br_x31 *= (transformer["NOMV3"] / _get_bus_value(bus_id3, "base_kv", pm_data))^2 * (pm_data["baseMVA"] / transformer["SBASE3-1"]) | ||
if transformer["NOMV2"] == 0.0 |
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.
same note as above.
src/io/psse.jl
Outdated
br_x23 *= (transformer["NOMV2"] / _get_bus_value(bus_id2, "base_kv", pm_data))^2 * (pm_data["baseMVA"] / transformer["SBASE2-3"]) | ||
end | ||
|
||
if transformer["NOMV3"] == 0.0 |
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.
same note as above
Looks good. Thanks for the contribution! |
* 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]>
This PR is to address the issue #912, which also partly addresses the issue #888. The issue #888 still stands, but with this PR, it solves by changing the transformer sbase to >70 (it is 60 now and not solving).
PSSE three-winding transformer supports
which are now supported in the PSSE parser as part of this PR.