[Improvement] Convert all supported elements' outputs with asymmetric load flow

[furqan463]

Changes proposed in this PR include:

The pandapower converter converts some elements like "shunt", "motor" and "ward" to pgm_input_data, but when output from pgm is converted back to pandapower with asymmetric load flow, these elements are not converted back. Resulting into a mismatch between power being injected by the source vs power being consumed/lost/inject by elements. Although Pandapower itself do not provide these output tables in case of 3ph power flow and the validation tests created here would fail if pf run with pandapower, but that do not stop us from improving the pgm_io.

Checks

• Implement res_shunt_3ph
• Implement res_motor_3ph
• Implement res_ward_3ph

[furqan463]

One thing that needs discussion is whether we should keep shunt & ward as balanced?
I think motor is clear, it's balanced. Same is true for ward as well but only confusion is that pandapower by default creates empty res_ward_3ph as unbalanced one, although newtork_structure.py do not define this structure.
For shunts, balanced is enough in my opinion.

[mgovers]

Great improvement once again! A couple remarks

Although Pandapower itself do not provide these output tables in case of 3ph power flow and the validation tests created here would fail if pf run with pandapower, but that do not stop us from improving the pgm_io.

Fully agree (c.c. @nitbharambe ). Maybe we can exclude those output tables from the validation check rather than just "assuming" that they are correct and then having the test fail?

[furqan463]

Maybe we can exclude those output tables from the validation check rather than just "assuming" that they are correct and then having the test fail?

Sorry I didn't get it.

[mgovers]

Maybe we can exclude those output tables from the validation check rather than just "assuming" that they are correct and then having the test fail?

Sorry I didn't get it.

apologies, i may also have misunderstood you.

@nitbharambe noticed that this PR may contain a breaking change (to be confirmed), as the pandapowerNet result class does not support those output types. I think that is also what you meant, right?

So we need to figure out a way to add the attributes while still remaining backwards compatibility. The following needs to be thought of:

• is the change even breaking? I.e., what would happen if:
  • we now output more and pp.runpp_pgm returns...
    • a different type (either derived or maybe something else)?
    • the same type with more attributes?
  • pandapower suddenly decides to also output those attributes...
    • with the same name?
    • but names them differently?

One possible solution @nitbharambe has thought of is to include a keyword argument in the PandaPowerConverter constructor. That way, pp.runpp_pgm() keeps the same type (in fact, we may add a keyword argument there to extend that behavior in the future), but the PandaPowerConverter supports both with and without the extra attributes. We will also need to check what happens if pandapower suddenly decides to also support these attributes (e.g.: we can use the ADict._allow_invalid_attributes attribute cf. https://github.com/e2nIEE/pandapower/blob/eea89af43e3e355d11bec19182143c138820fa63/pandapower/auxiliary.py#L194-L197 to check whether the attributes are supported, enabling forward compatibility)

I will add the do-not-merge label to keep track of this

[furqan463]

I think it'll not break anything, as PandaPowerNet is a dictionary, there's no result class, however, a file network_structure.py defines the structure of elements and result tables. However, PP is quite lenient with attributes, meaning users can add their own attributes to the tables and nothing breaks, the standard features just ignore those extra attributes however user can use those attributes when desired. similarly adding some extra result tables in PandaPowerNet dict should not cause any harm.

However, if pp adds these tables in future, the names of tables would be same as used here because these names are consistent with convention of the PP, the attributes of res_tables are also aligned.
The only thing that can change is what I've already pointed out in my first comment, whether pp defines these res_tables as symmetric i.e., have single p_mw, q_mvar or asymmetric i.e., having phase wise p_a_mw, p_b_mw, p_c_mw etc. but since res_load in case of asymmetric load flow generates symmetric output, so motor and ward would probably be symmetric.

Till the time pp does not include these results, the extra tables inside PandaPowerNet should not break anything, as these will be ignored.

[furqan463]

ADict._allow_invalid_attributes

This attribute does not restrict additional attributes, the name of this attribute is misleading, if you look at the code, it just controls the name of attribute being created, irrespective of that attribute being defined in network_structure.py or not.
In valid_name() it checked whether (1) name is a string (2) The key doesn't overlap with any class attributes (for Attr,
those would be 'get', 'items', 'keys', 'values', 'mro', and
'register').

ADict._allow_invalid_attributes = True, just let's you define attribute names that do not meet any one of the above criteria.

[furqan463]

However, we can define a flag to be passed to PandaPowerConverter, whether these extra tables should be included or not. and by default make it false.

[furqan463]

There's also another idea that I'm working on for my project, is to support ZIP load model for asym_load through PGM which is not implemented in PP.

[nitbharambe]

However, PP is quite lenient with attributes, meaning users can add their own attributes to the tables

I vaguely remember pandapower had that intention probably mentioned in their documentation or videos. But I searched their documentation now and couldn't find it.

Specifically this nets comparison function would give different results when runpp and runpp_pgm functions are used:

https://pandapower.readthedocs.io/en/v2.13.1/toolbox.html#comparison

Regarding this point:

The only thing that can change is what I've already pointed out in my first comment, whether pp defines these res_tables as symmetric i.e., have single p_mw, q_mvar or asymmetric i.e., having phase wise p_a_mw, p_b_mw, p_c_mw etc. but since res_load in case of asymmetric load flow generates symmetric output, so motor and ward would probably be symmetric.

We see that a res_load_3ph gets generated with only symmetric values for a symmetric load component for asymmetric calculation. Hence lets go with symmetric way as you suggested for both symmetric/asymmetric calculation.

Note: network_structure.py seems misleading. Its only used to create new empty network (and in their cim converter). The empty res_storage_3ph structure is different from the actual res_storage_3ph which contains symmetric form of result: p_mw and q_mvar.

[furqan463]

Note: network_structure.py seems misleading. Its only used to create new empty network (and in their cim converter). The empty res_storage_3ph structure is different from the actual res_storage_3ph which contains symmetric form of result: p_mw and q_mvar.

and for io as well.

[furqan463]

I vaguely remember pandapower had that intention probably mentioned in their documentation or videos. But I searched their documentation now and couldn't find it.

Not sure about documentation. But you can test it. You can add custom attributes to your tables.

[furqan463]

Specifically this nets comparison function would give different results when runpp and runpp_pgm functions are used

pp.toolbox.nets_equal already returns False for many reasons. But yes there'll be an extra warning message with False result saying:
logger.warning("These keys were ignored by the comparison of the networks: %s" % (', '.join(
not_checked_keys)))

However, if otherwise make two nets equal, nets_equal() will return True along with above message.
https://github.com/e2nIEE/pandapower/blob/eea89af43e3e355d11bec19182143c138820fa63/pandapower/toolbox/comparison.py#L140

That's why I say, PP is lenient with attributes.

[mgovers]

Specifically this nets comparison function would give different results when runpp and runpp_pgm functions are used

pp.toolbox.nets_equal already returns False for many reasons. But yes there'll be an extra warning message with False result saying: logger.warning("These keys were ignored by the comparison of the networks: %s" % (', '.join( not_checked_keys)))

However, if otherwise make two nets equal, nets_equal() will return True along with above message. https://github.com/e2nIEE/pandapower/blob/eea89af43e3e355d11bec19182143c138820fa63/pandapower/toolbox/comparison.py#L140

That's why I say, PP is lenient with attributes.

alright, thank you very much for the elaborate investigation and write-up. you've convinced me. @nitbharambe any open questions from your end?

[nitbharambe]

Okay sounds good. No more open questions from me.

An observation: I see res_ward has a vm_pu column as output. There would be no reasonable way to have that in res_ward_3ph unless mentioned for each phase individually. Hence lets not give out that output (ie. as is currently done in this PR.)

[furqan463]

@mgovers @nitbharambe there are some changes staged for next release of PP which will require change is PGM-IO, as const_i_percent and const_z_percent parameters of load will not be available in next release. We can't make these changes now, but may be create an issue about it, just a heads up.

[mgovers]

@mgovers @nitbharambe there are some changes staged for next release of PP which will require change is PGM-IO, as const_i_percent and const_z_percent parameters of load will not be available in next release. We can't make these changes now, but may be create an issue about it, just a heads up.

Good find! Definitely is a breaking change on our end. Can you please make an issue?

A potential solution that is also backwards compatible could be to check for the pandapower version as we've done in the past as well.

[furqan463]

Can you please make an issue?

Done.

[mgovers]

I prefer not to do changes like this right before the weekend. Otherwise, I agree that this is good to go. Let's merge this coming Monday.

[mgovers]

Happy Monday!