Skip to content

Complete persistent iidm implementation #163

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

Merged
merged 37 commits into from
Feb 9, 2021

Conversation

SlimaneAmar
Copy link
Contributor

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, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

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)

SlimaneAmar added a commit that referenced this pull request Dec 11, 2020
@SlimaneAmar SlimaneAmar requested a review from geofjamg December 11, 2020 16:01
SlimaneAmar pushed a commit that referenced this pull request Dec 17, 2020
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 5bcc0ce to de0f761 Compare December 17, 2020 08:31
SlimaneAmar added a commit that referenced this pull request Dec 17, 2020
Signed-off-by: Slimane AMAR <[email protected]>
SlimaneAmar added a commit that referenced this pull request Dec 21, 2020
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 04da2ad to 331a6e5 Compare December 21, 2020 11:59
SlimaneAmar added a commit that referenced this pull request Dec 21, 2020
SlimaneAmar added a commit that referenced this pull request Dec 21, 2020
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from f2dc293 to b897899 Compare December 21, 2020 17:25
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from b897899 to 8283cab Compare December 21, 2020 17:45
SlimaneAmar added a commit that referenced this pull request Dec 23, 2020
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 0791533 to 7e339f0 Compare December 23, 2020 11:40
SlimaneAmar added a commit that referenced this pull request Dec 30, 2020
…mically created on the fly from attributes (#163)

Signed-off-by: Slimane AMAR <[email protected]>
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 19906aa to 9544d1c Compare February 4, 2021 18:11
@SlimaneAmar SlimaneAmar requested a review from niconoir February 5, 2021 11:10
SlimaneAmar added a commit that referenced this pull request Feb 5, 2021
Signed-off-by: Slimane AMAR <[email protected]>
Signed-off-by: Slimane AMAR <[email protected]>
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 7ed5216 to d9b92ff Compare February 5, 2021 12:29
SlimaneAmar added a commit that referenced this pull request Feb 5, 2021
Signed-off-by: Slimane AMAR <[email protected]>
SlimaneAmar added a commit that referenced this pull request Feb 5, 2021
Signed-off-by: Slimane AMAR <[email protected]>
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 2b0c3db to 15b0312 Compare February 5, 2021 17:18
SlimaneAmar added a commit that referenced this pull request Feb 5, 2021
Signed-off-by: Slimane AMAR <[email protected]>
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 15b0312 to 3b49ed6 Compare February 5, 2021 19:15
Signed-off-by: Slimane AMAR <[email protected]>
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 3b49ed6 to 2a684ba Compare February 8, 2021 07:47
@SlimaneAmar SlimaneAmar requested a review from niconoir February 8, 2021 07:47

Graph<Integer, Edge> graph = NodeBreakerTopology.INSTANCE.buildGraph(index, voltageLevelResource);
Graph<Integer, Edge> graph = NodeBreakerTopology.INSTANCE.buildGraph(index, voltageLevelResource, true, true);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this method called with isBusView to true. What is the purpose of this option?

Copy link
Contributor Author

@SlimaneAmar SlimaneAmar Feb 8, 2021

Choose a reason for hiding this comment

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

The connectivity graph is used to calculate buses in a VL and in node/breaker the calculated buses depends on the chosen view.
BusView : !switch::isOpen
BusBreakerView : !switch::isOpen && !switch::isRetained
But when we just traverse the topology, we do not take into account the switches (open or retained).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe i need rename isBusView by includeRetainedSwitches

}

@Override
public void remove() {
resource.getAttributes().setConverterStationId1(null);
Copy link
Member

Choose a reason for hiding this comment

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

Why do yu need to change the resource as it won't be used anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree but the tck test AbstractLccTest::testHvdcLineRemove considers that it must be set to null and checks it

@@ -27,16 +27,19 @@ static HvdcLineImpl create(NetworkObjectIndex index, Resource<HvdcLineAttributes

@Override
public HvdcConverterStation<?> getConverterStation1() {
return index.getHvdcConverterStation(resource.getAttributes().getConverterStationId1()).orElseThrow(IllegalStateException::new);
return resource.getAttributes().getConverterStationId1() != null ? index.getHvdcConverterStation(resource.getAttributes().getConverterStationId1()).orElse(null) : null;
Copy link
Member

Choose a reason for hiding this comment

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

Is should not be possible to have this null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment above

}

@Override
public HvdcConverterStation<?> getConverterStation2() {
return index.getHvdcConverterStation(resource.getAttributes().getConverterStationId2()).orElseThrow(IllegalStateException::new);
return resource.getAttributes().getConverterStationId2() != null ? index.getHvdcConverterStation(resource.getAttributes().getConverterStationId2()).orElse(null) : null;
Copy link
Member

Choose a reason for hiding this comment

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

Same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see comment above

public Collection<Component> getConnectedComponents() {
throw new UnsupportedOperationException("TODO");
public Collection<Component> getConnectedComponents() { // FIXME : need a reference bus by component
return getBusStream().map(Bus::getConnectedComponent).collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

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

This is not correct, as you are going to have here as many ConnectedComponent object as Bus but the aim was to get a list of unique ConnectedComponent

…or optimization purposes (#163)

Signed-off-by: Slimane AMAR <[email protected]>
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 6d28ea0 to 772a17d Compare February 9, 2021 11:56
@SlimaneAmar SlimaneAmar force-pushed the complete_persistent_iidm_implementation branch from 772a17d to a0fb7f9 Compare February 9, 2021 12:08
@SlimaneAmar SlimaneAmar requested a review from geofjamg February 9, 2021 12:21
@SlimaneAmar SlimaneAmar changed the title [WIP] Complete persistent iidm implementation Complete persistent iidm implementation Feb 9, 2021
# Conflicts:
#	network-store-server/src/main/java/com/powsybl/network/store/server/NetworkStoreRepository.java
#	network-store-server/src/main/resources/iidm.cql
Signed-off-by: Geoffroy Jamgotchian <[email protected]>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2021

Kudos, SonarCloud Quality Gate passed!

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

85.4% 85.4% Coverage
0.4% 0.4% Duplication

@geofjamg geofjamg merged commit 2d5a637 into master Feb 9, 2021
@geofjamg geofjamg deleted the complete_persistent_iidm_implementation branch February 9, 2021 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants