Closed awig closed 7 years ago
Sounds good. It would be useful if you let me know some of the details of your plan a bit ahead of time so we can start a discussion. I am guessing you will start with changing the naming convention (bus_k instead of bus_from?)
Yes. Naming convention first and then integrating it although I think I would do just one PR to dev here. Here is what I'm proposing for the naming:
constr_PF.c
flowP —> Pvkvm
flowQ —> Qvkvm
flowP_sh —> Pvkvk
flowQ_sh —> Qvkvk
elsewhere (net.c/branch.c)
flowP —> P_series_bus_from, P_series_bus_to
flowQ —> Q_series_bus_from, P_series_bus_to
flowP_sh —> P_shunt_bus_from, P_shunt_bus_to
flowQ_sh —> Q_shunt_bus_from, Q_shunt_bus_to
*new* P_bus_from, P_bus_to
*new* Q_bus_from, Q_bus_to
@ttinoco what do you think?
For constr_PF.c: I see no need to include "v". How about:
flowP -> Pkm
flowP_sh -> Pkk
and similar for Q. These are just local variables in constr_PF.c so these changes are easy.
For net.c and branch.c we should first discuss the branch flow routines that will be added. This is much more important that the names of the local variables in net.c (update_properties). I was thinking of something like
BRANCH_get_Pkm
BRANCH_get_Pkm_series
BRANCH_get_Pkm_shunt
BRANCH_get_Pmk
BRANCH_get_Pmk_series
BRANCH_get_Pmk_shunt
The branch should not have structure fields for these flows since they are derived from the bus/branch data and var values. These routines should probably take a Vec* argument so that they can be used in NET_update_properties (let me know if you have questions about this). These BRANCH routines would force changing the BRANCH struct fields "bus_from" and "bus_to" to "bus_k" and "bus_m" respectively. These changes will require updating many parts of the C and Python code (e.g. code that calls BRANCH_get_bus_from to code that calls BRANCH_get_bus_k, etc). I think these name changes using k and m instead of "from" and "to" are required. The ones you suggested are too long in my opinion.
Once we take care of these BRANCH flow "getters" then we can update the code in net.c to use them.
Sure. No problem.
@martinzellner gave me a great suggestion today: we should change the branch field names (e.g. bus_from to bus_k) and add the new interface routines (get_bus_k, get_Pkm, etc) but KEEP the old interface routines (e.g. get_bus_from) and mark them as DEPRECATED (so that old code does not just break)
Agreed. I will include these changes as part of this issue.
For consistency should we use an underscore to separate the subscript in all cases (at least going forward)? I notice that other fields like 'P_max' and 'v_mag' for example always separate the subscript from the identifying symbol. Thus, I would change to P_km, b_k, b_m etc.
Style question: should the old interface routines be modified to just use the new fields or call the new routines?
Your subscript suggestion is good. What do you mean by b_k and b_m? What are these variables? Do you mean bus_k and bus_m? With regards to style, either way would be ok but I am leaning towards "call the new routines"
I think calling the new routine looks cleanest, so I'll go with that.
In branch.c their are fields for the shunt conductance and susceptance as viewed from both k and m sides (currently b_from, b_to, g_from, g_to which I would change to b_k, b_m, g_k, g_m respectively, as well as the names of the branch list objects from_next, to_next to k_next, m_next respectively.
Ok, I remember now. Thanks. Should we use "next_k"? instead of "k_next"?
Yes! that reads better.
Quick update: I believe I have implemented all the name changes and have put place holders for the BRANCH_get_Pkm etc. Everything compiles and I can reference the new and old naming structure. I may have some questions about how the vector of var_values is used but need to spend more time understanding it.
Testing on Romcon/PFNET and will do a PR here afterwards.
Issue has been taken care of in latest pull request
I will be adding branch flows to the branch struct and will perform the calculation of the branch flows within the struct as well. These flows will then be used by the update_properties in net.c. This will NOT change constr_PF.c.
I will also change the naming conventions to clarify what elements of the branch and from what bus the flows are being calculated.