Two windings transformer: add phaseAngleClock, only one tap changer regulating.

[marqueslanauja]

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

• The commit message follows our guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature

What is the current behavior? (You can also link to an open issue here)
More than one tap changer regulating control enabled is allowed (ratio and phase)

What is the new behavior (if this is a feature change)?
Only one tap changer regulating control enabled is allowed
PhaseAngleClock is added in twoWindingsTransformer as an extension

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:

• The Breaking Change or Deprecated label has been added
• The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:

(if any of the questions/checkboxes don't apply, please delete them entirely)
Some tests has been adjusted as they have twoWindingsTransformers with more than one tapChanger control enabled.

[coveralls]

Coverage increased (+0.02%) to 84.291% when pulling a96f59d on cgmes_conversion_T2x into 2844971 on master.

[zamarrenolm]

We could add a test to check the set method in the phase angle clock extension, to increase coverage

[zamarrenolm]

sonar warning about duplicated code is related to common methods with other XML serializers. It is not relevant.

One option to fix the duplicated code would be to set the name of the XSD resource, the namespace URI and the namespace prefix as attributes at ExtensionXmlSerializer, so methods to obtain these items can be implemented at ExensionXmlSerializer, without the need to override at every child class.

[miovd]

sonar warning about duplicated code is related to common methods with other XML serializers. It is not relevant.

One option to fix the duplicated code would be to set the name of the XSD resource, the namespace URI and the namespace prefix as attributes at ExtensionXmlSerializer, so methods to obtain these items can be implemented at ExensionXmlSerializer, without the need to override at every child class.

For now, we consider this as false positive, it will not block the merging into master for us.

[geofjamg]

Should it be flagged as breaking change (and added to migration guide) as for instance load flow impl have to be modified to take this attribute into account?

[miovd]

Should it be flagged as breaking change (and added to migration guide) as for instance load flow impl have to be modified to take this attribute into account?

I am not sure this PR qualifies as Breaking Change since no breaking change is explicitly made here... Behavior is not modified anywhere by this PR. Maybe in another PR, a LF parameter indicating if phase angle clocks are ignored or not will be added and this PR should then be declared as broken change but not this one in my opinion.

[zamarrenolm]

Should it be flagged as breaking change (and added to migration guide) as for instance load flow impl have to be modified to take this attribute into account?

I am not sure this PR qualifies as Breaking Change since no breaking change is explicitly made here... Behavior is not modified anywhere by this PR. Maybe in another PR, a LF parameter indicating if phase angle clocks are ignored or not will be added and this PR should then be declared as broken change but not this one in my opinion.

I agree with Miora. Phase angle clock values are relevant for short-circuit analysis and load flow calculations can safely ignore them (voltages and flows obtained will be the same with or without these values).

Some TSOs take them into account in their load flow calculations, but it is not a requirement. In these cases load flow results angles have a phase shift in some buses (after the transformer end that has the phase angle clock). As these buses are not connected to other parts of the network, this phase shift has no real impact on the electric analysis using load flow. Storing phase angle clocks has been introduced mainly to be able to do comparisons in these situations.

Next pull request related to this set of changes will be a breaking change, as it will introduce the new three winding transformer API.

[geofjamg]

Should it be flagged as breaking change (and added to migration guide) as for instance load flow impl have to be modified to take this attribute into account?

I am not sure this PR qualifies as Breaking Change since no breaking change is explicitly made here... Behavior is not modified anywhere by this PR. Maybe in another PR, a LF parameter indicating if phase angle clocks are ignored or not will be added and this PR should then be declared as broken change but not this one in my opinion.

I agree with Miora. Phase angle clock values are relevant for short-circuit analysis and load flow calculations can safely ignore them (voltages and flows obtained will be the same with or without these values).

Some TSOs take them into account in their load flow calculations, but it is not a requirement. In these cases load flow results angles have a phase shift in some buses (after the transformer end that has the phase angle clock). As these buses are not connected to other parts of the network, this phase shift has no real impact on the electric analysis using load flow. Storing phase angle clocks has been introduced mainly to be able to do comparisons in these situations.

Next pull request related to this set of changes will be a breaking change, as it will introduce the new three winding transformer API.

Ok thanks.