Add creation of Y matrix from Network, multiphase breakout#3
Add creation of Y matrix from Network, multiphase breakout#3zzink731 wants to merge 1 commit intoNLaws:mainfrom
Conversation
NLaws
left a comment
NLaws
left a comment
There was a problem hiding this comment.
Thanks for doing this! Totally understand if you don't want to address the requested changes if this meets your needs. The high level summary is that the Y builder should live in CommonOPF.
| @@ -0,0 +1,62 @@ | |||
|
|
|||
|
|
|||
| function admittance_builder(net::Network{MultiPhase}) | |||
There was a problem hiding this comment.
This method should live in CommonOPF (according to the design patterns I'm working within, not something I expect you to be aware of). This way there is no need to add the Graphs library to BIM.jl and the admittance_builder is available to any package that uses CommonOPF.
There was a problem hiding this comment.
Sounds good! Happy to take direction on specifically where you want these things to live.
| function admittance_builder(net::Network{MultiPhase}) | ||
|
|
||
| order = bfs_order(net) | ||
| Y = zeros(ComplexF64, (3*length(order), 3*length(order))) |
There was a problem hiding this comment.
It would be best to use a sparse matrix for Y (because it is mostly zeros and gets big for real networks).
| non_zero_indices = findall(non_zero_rows) .|> x -> x[1] | ||
| Y = Y[non_zero_indices, non_zero_indices] | ||
|
|
||
| return Y |
There was a problem hiding this comment.
how do you track the order of Y's indices? One thought I had on this would be to use an AxisArray, which can be indexed on strings (i.e. bus names) but then Y cannot be a sparse matrix. So I think that it's best to use Y like OpenDSS does, with a "node order" vector of strings as a companion to Y. Then one could index into Y like
bus_to_node = Dict(b => i for (b, i) in enumerate(node_order))
Y[bus_to_node["bus1"], bus_to_node["bus2"]]There was a problem hiding this comment.
I was thinking that bfs order of busses does provide an unambiguous general format but wouldn't give an indication of which phases exist at each node. So, yes, I like node_order. Do you like the OpenDSS format? e.g. 650.1, 650.2, 650.3...
| end | ||
|
|
||
| function bfs_order(net::Network{MultiPhase}) | ||
| b = Graphs.bfs_tree(net.graph, 1) |
There was a problem hiding this comment.
This is the source vertex to make the tree from. I am assuming the source of net.graph is always at 1 but perhaps this is not the case?
There was a problem hiding this comment.
it's unclear because we use MetaGraphsNext to make the net.graph. I think that the following extension will meet the need:
Graphs.bfs_tree(net::Network, bus::String) = Graphs.bfs_tree(net.graph, net.graph[][:bus_int_map][bus])I did something like this here, which is a good spot to put the bfs_tree extension.
|
|
||
| for edge in CommonOPF.edges(net) | ||
|
|
||
| z_prim = CommonOPF.zij(edge[1],edge[2],net) |
There was a problem hiding this comment.
why not use CommonOPF.yij (and skip the inverse step) ?
There was a problem hiding this comment.
Oh nice, I didn't see this in that commit you sent over and my local CommonOPF I was looking in was not current. I definitely will.
There was a problem hiding this comment.
v0.4.3 of CommonOPF was just released today (went through some issues with the OpenDSSDirect dependency).
| include("single_phase_model.jl") | ||
| include("single_phase_fpl_model.jl") | ||
| include("multi_phase_fpl_model.jl") | ||
| include("utilities.jl") |
There was a problem hiding this comment.
No need for this new file include (see comment about putting the Y builder in CommonOPF).
2 participants
Hi Nick,
Went ahead and added that Y matrix creation you alluded to. I also started to separate single-phase and multi-phase. I didn't touch single_phase_model.jl, however, even though it calls for
net::Network{MultiPhase}Cheers,
Zephyr