Skip to content
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

Reference priorities and reference terminals iIDM extensions #2763

Merged
merged 35 commits into from
Nov 22, 2023

Conversation

jeandemanged
Copy link
Member

@jeandemanged jeandemanged commented Nov 2, 2023

Please check if the PR fulfills these requirements

  • 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?

#2747 in part. I propose this first PR about the iIDM extensions. Once validated/merged, other PRs will be issued for the various importers/exporter (CGMES but not only).

What kind of change does this PR introduce?

Feature

What is the current behavior?

Today we only have the SlackTerminal extension.
Cannot distinguish from Slack Terminal (where slack power is distributed) to Reference Terminals (where the angle reference is).

What is the new behavior (if this is a feature change)?
2 new iIDM extensions:

  • ReferencePriorities: Intended to serve as an input for Load Flow calculations. Placed on Connectables, allows specifying priority for selection as angle reference (same principle as cim:SynchronousMachine.referencePriority in CIM/CGMES, but generalized to any Terminal).
  • ReferenceTerminals: Intended to serve as an output for Load Flow calculations. Placed on Network, allows storing which terminals we used as angle reference (normally one per calculated synchronous component). In CIM/CGMES context, the Bus of these Terminals could be used to compute cim:TopologicalIsland.angleRefTopologicalNode in SV.

Both extentions support multi-variant, and XML serialization.

Does this PR introduce a breaking change or deprecate an API?

No

Other information:

again: once this PR validated/merged, other PRs will be issued for the various importers/exporter (CGMES but not only).

Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
…ferencePriority

Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
@jeandemanged jeandemanged changed the title [WIP] Reference terminal extension [WIP] Reference priorities extension Nov 6, 2023
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
@jeandemanged jeandemanged changed the title [WIP] Reference priorities extension [WIP] Reference priorities and reference terminals extensions Nov 6, 2023
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
@jeandemanged jeandemanged changed the title [WIP] Reference priorities and reference terminals extensions Reference priorities and reference terminals extensions Nov 7, 2023
@jeandemanged jeandemanged changed the title Reference priorities and reference terminals extensions Reference priorities and reference terminals iIDM extensions Nov 7, 2023
@geofjamg geofjamg self-requested a review November 7, 2023 15:29
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Copy link
Member

@geofjamg geofjamg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor comments

*/
class ReferenceTerminalsImpl extends AbstractMultiVariantIdentifiableExtension<Network> implements ReferenceTerminals {

private final class ReferenceTerminalsListener extends DefaultNetworkListener {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very interesting way to solve the terminal removal cleanup issue. I think we have a bug in SlackTerminal implementation...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes SlackTerminal, also RemoteReactivePowerControl (but this one will be part of iIDM one day), maybe others, to be reviewed and fixed in other PRs.

Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
This reverts commit eb47953.

Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
@jeandemanged jeandemanged force-pushed the reference_terminal_extension branch from ac77713 to c4d733d Compare November 14, 2023 17:53
jeandemanged and others added 3 commits November 15, 2023 11:38
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
jeandemanged and others added 4 commits November 21, 2023 11:52
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
# Conflicts:
#	iidm/iidm-serde/src/main/java/com/powsybl/iidm/serde/extensions/ReferencePrioritiesXmlSerializer.java
#	iidm/iidm-serde/src/main/java/com/powsybl/iidm/serde/extensions/ReferenceTerminalsXmlSerializer.java
#	iidm/iidm-serde/src/main/resources/xsd/referencePriorities.xsd
#	iidm/iidm-serde/src/main/resources/xsd/referenceTerminals.xsd
#	iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/extensions/ReferencePrioritiesXmlTest.java
#	iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/extensions/ReferenceTerminalsXmlTest.java
#	iidm/iidm-serde/src/test/resources/V1_11/referencePrioritiesRef.xiidm
#	iidm/iidm-serde/src/test/resources/V1_11/referenceTerminalsRef.xiidm
Copy link
Member

@annetill annetill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just needed an ajustement to new artifactId for serialization.
This PR is okay and great, but:

  • We have now a difference in slack terminals management and in reference terminals management. A refacotring of slack terminals management is maybe needed in the future.
  • We don't have any utility method like ReferenceTerminal.attach(bus). It is the best one of slack terminal after a power flow engine!

Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

96.8% 96.8% Coverage
0.0% 0.0% Duplication

@flo-dup flo-dup merged commit 1466d17 into main Nov 22, 2023
@flo-dup flo-dup deleted the reference_terminal_extension branch November 22, 2023 10:26
zamarrenolm referenced this pull request Mar 7, 2024
Signed-off-by: Romain Courtier <romain.courtier@rte-france.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants