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.

Sign up for GitHub

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

Jump to bottom

New "Ground" injection #2837

Merged
merged 31 commits into from
Jan 15, 2024
Merged

New "Ground" injection #2837

merged 31 commits into from
Jan 15, 2024

Conversation

rolnico
Copy link
Member

@rolnico rolnico commented Dec 18, 2023 •
edited by olperr1
Loading

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?
#1131
But, in fact, ground disconnectors are not ignored but classified as disconnectors.

See for information first try in #2757

What kind of change does this PR introduce?
Bug fix / feature

What is the current behavior?
No distinction is made between ground disconnectors and disconnectors in IIDM and no ground element exists

What is the new behavior (if this is a feature change)?
A new Ground identifiable is created and can be added next to a disconnector to modelise a "ground disconnector"

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

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

If you defined your own IIDM implementation, you should implement the following methods:

  • in your Network implementations:

    • Iterable<Ground> getGrounds()
    • Stream<Ground> getGroundStream()
    • int getGroundCount()
    • Ground getGround(String id)

  • in your VoltageLevel implementation:

    • GroundAdder newGround()
    • Iterable<Ground> getGrounds()
    • Stream<Ground> getGroundStream()
    • int getGroundCount()

  • in your TopologyVisitor implementation:

    • void visitGround(Ground connectable)

Other information:

flo-dup and others added 11 commits December 4, 2023 13:34
@flo-dup
Add 1.12 xsd, IidmVersion and set current version to 1.12
fc40d6d 
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup
Create 1.12 test references for versioned unit tests
0009bab 
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup
Create 1.12 json unit test references
946210a 
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup
Create iidm equipment unit test reference
145f693 
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup
Update non-versioned unit tests
55fe19f 
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup
Update AbstractVersionableNetworkExtensionSerDe classes
dffd0db 
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@annetill
Merge branch 'main' into evolution_xiidm/1.12
0b7b83b
@rolnico
create Ground Identifiable
72f0785 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
checkstyle
80f7674 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
typo in test
16d875d 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
improved coverage
b172ef0 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@annetill annetill changed the base branch from main to evolution_xiidm/1.12 December 19, 2023 11:23
@rolnico
test coverage
2a18545 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
checkstyle
d12ae22 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@annetill annetill mentioned this pull request Dec 19, 2023
Closed
5 tasks
@rolnico
new implementation + SerDe
7258c98 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
merged switch
e73fa5e 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico rolnico changed the title [WIP] New "Ground" indentifiable New "Ground" indentifiable Dec 21, 2023
@rolnico
add TCK test
6862d28 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico rolnico marked this pull request as ready for review December 21, 2023 08:42
Base automatically changed from evolution_xiidm/1.12 to main January 10, 2024 12:07
So-Fras
So-Fras requested changes Jan 10, 2024
View reviewed changes
Copy link
Member

@So-Fras So-Fras left a comment

Choose a reason for hiding this comment

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

Thnak you for this PR.
I left 2 minor remarks.
In order to make the examples in tests more realistic, the disconnectors (or the bus in bus-breaker view) associated with the grounds could be linked with a line.

iidm/iidm-api/src/main/java/com/powsybl/iidm/network/Network.java Outdated

/**
* Get an equipment by its ID or alias
* * Get an equipment by its ID or alias
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* * Get an equipment by its ID or alias
* Get an identifiable by its ID or alias

(equipment is uncountable + * was duplicated)

NB: there is a similar typo on the following line. I suggest to replace equipment by identifiable there too.

iidm/iidm-tck/src/test/java/com/powsybl/iidm/network/tck/AbstractGroundTest.java
private Network network;

@BeforeEach
public void initNetwork() {
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this network is a duplicate of the network created in GroundSerDeTest, at least if we omit the ground and the associated disconnector parts. If it is indeed the case, could it be possible to factorize this code in the iidm-test package?

@So-Fras
Copy link
Member

So-Fras commented Jan 10, 2024

I am thinking about something: should we allow the ground creation in bus/breaker view? Will the associated disconnector be present on a bus/breaker network?

@rolnico
replace equipment with identifiable in javadoc
76735b9 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
network creation moved to iidm-test to avoid duplication
0aba02d 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
delete old and now unused network creation method
13a9370 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
delete comment
b53fb3e 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
Merge branch 'main' into nro/ground_disconnector
5b6a4e2 
# Conflicts:
#	iidm/iidm-serde/src/main/resources/xsd/iidm_V1_12.xsd
#	iidm/iidm-serde/src/main/resources/xsd/iidm_equipment_V1_12.xsd
#	iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/TerminalMockSerDe.java
#	iidm/iidm-serde/src/test/resources/V1_12/branchObservabilityRoundTripRef.xml
#	iidm/iidm-serde/src/test/resources/V1_12/completeThreeWindingsTransformerRoundTripRef.xml
#	iidm/iidm-serde/src/test/resources/V1_12/coordinatedReactiveControl.xml
#	iidm/iidm-serde/src/test/resources/V1_12/danglingLine.xml
#	iidm/iidm-serde/src/test/resources/V1_12/disMeasRef.xiidm
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-fict-inj.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-loading-limits.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial-example1-anonymized.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial-example1-expo-load-model.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial-example1-opt-sub.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial-example1-properties.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial-example1-with-terminalMock-ext.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial-example1-zip-load-model.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial-example1.xml
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial1-lf-extensions.json
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial1-lf.json
#	iidm/iidm-serde/src/test/resources/V1_12/eurostag-tutorial1-lf.xml
#	iidm/iidm-serde/src/test/resources/V1_12/faultyThreeWindingsTransformerRoundTripRef.xml
#	iidm/iidm-serde/src/test/resources/V1_12/measRef.xiidm
#	iidm/iidm-serde/src/test/resources/V1_12/secondaryVoltageControlRoundTripRef.xml
#	iidm/iidm-serde/src/test/resources/V1_12/subnetworks.xml
#	iidm/iidm-serde/src/test/resources/V1_12/substationAndLinePositionRoundTripRef.xml
#	iidm/iidm-serde/src/test/resources/V1_12/t3w-loading-limits.xml
#	iidm/iidm-serde/src/test/resources/V1_12/terminalRef.xiidm
#	iidm/iidm-serde/src/test/resources/V1_12/threeWindingsTransformerPhaseAngleClock.xml
#	iidm/iidm-serde/src/test/resources/V1_12/threeWindingsTransformerRoundTripRef.xml
#	iidm/iidm-serde/src/test/resources/V1_12/threeWindingsTransformerToBeEstimated.xiidm
#	iidm/iidm-serde/src/test/resources/V1_12/tieLineTerminalRefBug.xml
#	iidm/iidm-serde/src/test/resources/V1_12/tieline.xml
#	iidm/iidm-serde/src/test/resources/V1_12/tielineWithAliases.xml
#	iidm/iidm-serde/src/test/resources/V1_12/tl-loading-limits.xml
#	iidm/iidm-serde/src/test/resources/V1_12/twoWindingsTransformerPhaseAndRatioTap.xml
#	iidm/iidm-serde/src/test/resources/V1_12/twoWindingsTransformerPhaseAndRatioTapWithExtensions.xml
#	iidm/iidm-serde/src/test/resources/V1_12/twoWindingsTransformerPhaseAngleClock.xml
#	iidm/iidm-serde/src/test/resources/V1_12/twoWindingsTransformerToBeEstimated.xiidm
#	iidm/iidm-serde/src/test/resources/V1_12/voltageAngleLimit.xiidm
#	iidm/iidm-serde/src/test/resources/V1_12/xsd/terminalMock.xsd
@zamarrenolm zamarrenolm mentioned this pull request Jan 11, 2024
Merged
3 tasks
rolnico and others added 7 commits January 11, 2024 16:34
@rolnico
factorisation of a variable and a few methods in AbstractInjectionAdder
4ce93a9 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
allow setFictitious if false only
239d589 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
delete ground from list in isValid
8ff3086 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
Merge branch 'main' into nro/ground_disconnector
62db5dd 
# Conflicts:
#	iidm/iidm-api/src/main/java/com/powsybl/iidm/network/IdentifiableType.java
@rolnico
add ground from list in isValid with right exit
5fcdc3e 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@flo-dup
Merge branch 'main' into nro/ground_disconnector
f329af7
@rolnico
delete getVariantManagerHolder method
997b7a2 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@flo-dup flo-dup requested a review from So-Fras January 15, 2024 11:25
flo-dup
flo-dup requested changes Jan 15, 2024
View reviewed changes
Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

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

Just realized we forgot to put it in TopologyVisitor!

@rolnico
add visitGround method in visitor + added Ground in some remaining sw…
1bcfb12 
…itch cases

Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

3 New issues
0 Security Hotspots
95.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@flo-dup flo-dup changed the title New "Ground" indentifiable New "Ground" identifiable Jan 15, 2024
@flo-dup flo-dup changed the title New "Ground" identifiable New "Ground" injection Jan 15, 2024
@flo-dup flo-dup merged commit 47f57b3 into main Jan 15, 2024
6 checks passed
@flo-dup flo-dup deleted the nro/ground_disconnector branch January 15, 2024 15:58
@olperr1 olperr1 added the Breaking Change API is broken label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@So-Fras So-Fras So-Fras approved these changes

@flo-dup flo-dup flo-dup approved these changes

@geofjamg geofjamg Awaiting requested review from geofjamg

Assignees
No one assigned
Labels
Breaking Change API is broken IIDM January release new version XIIDM/JIIDM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rolnico @So-Fras @flo-dup @olperr1 @annetill