fix syncv #460

jonenst · 2024-09-27T16:14:37Z

No description provided.

Signed-off-by: Franck LECUYER <[email protected]>

Add new test with multiple buses in bus view and bus breaker view in a voltage level Signed-off-by: Franck LECUYER <[email protected]>

Signed-off-by: Franck LECUYER <[email protected]>

Signed-off-by: HARPER Jon <[email protected]>

jonenst · 2024-09-30T10:48:54Z

example fixes for #452

Signed-off-by: Franck LECUYER <[email protected]>

…tWithSwitchTest, with buses in bus breaker view between retained switches and with no equipments. Signed-off-by: Franck LECUYER <[email protected]>

Signed-off-by: Franck LECUYER <[email protected]>

jonenst · 2024-10-22T08:55:07Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/AbstractTopology.java

-            if (foundConfiguredBus.get() != null) {
-                v.set(foundConfiguredBus.get().getResource().getAttributes().getV());
-                angle.set(foundConfiguredBus.get().getResource().getAttributes().getAngle());
+            if (foundConfiguredBus.get() == null) {


break early instead of checking if found in the loop

jonenst · 2024-10-22T09:23:30Z

network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/VoltageLevelTest.java

+        LineImpl l1 = (LineImpl) network.getLine("L1");
+
+        // Update angle using BusView
+        l1.getTerminal1().getBusView().getBus().setAngle(111);


for better coverage, start the angle busbreaker topology test with a setAngle in the busbreakerview
and the nodebreaker topology test start with a setAngle in the busview

to do the opposite of the setV test

jonenst · 2024-10-22T09:43:48Z

...-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVTest.java

+            String busbreakerviewbusid = entry.getKey();
+            String busviewbusid = entry.getValue();
+
+            for (Bus bbvb : vl.getBusBreakerView().getBuses()) {


we need to refactor these test to allow testing with or without already calculated buses. Here because we force getBuses before the tests, we always run with calculated bus already computed

jonenst · 2024-10-22T12:15:03Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/AbstractTopology.java

+                                           AtomicDouble v,
+                                           AtomicDouble angle,
+                                           boolean isBusView) {
+        if (voltageLevelResource.getAttributes().getTopologyKind() == TopologyKind.NODE_BREAKER) {


should we use if (isCalculatedBusesValid(voltageLevelResource, !isBusView)) {
like in getCalculatedBusAttributesList ?

…istence Signed-off-by: HARPER Jon <[email protected]>

sonarcloud · 2024-10-22T16:52:27Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jonenst · 2024-12-04T23:12:10Z

@FranckLecuyer can you take a look ?

…ws, simplify code, throw exceptions instead of silently doing nothing Signed-off-by: HARPER Jon <[email protected]>

sonarcloud · 2024-12-04T23:24:05Z

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

antoinebhs

Quick reading of this PR, no tests

antoinebhs · 2024-12-05T08:03:10Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/AbstractTopology.java

+    private CalculatedBusAttributes createCalculatedBusAttributesWithVAndAngle(NetworkObjectIndex index,
+                                           Resource<VoltageLevelAttributes> voltageLevelResource,
+                                           ConnectedSetResult<T> connectedSet,
+                                           boolean isBusView) {
+        double v = Double.NaN;
+        double angle = Double.NaN;
+        if (voltageLevelResource.getAttributes().getTopologyKind() == TopologyKind.NODE_BREAKER) {
+            CalculatedBusAttributes busAttributes = findFirstMatchingNodeBreakerCalculatedBusAttributes(voltageLevelResource, connectedSet, isBusView);
+            if (busAttributes != null) {
+                v = busAttributes.getV();
+                angle = busAttributes.getAngle();
+            }
+        } else { // BUS_BREAKER
+            // currently for busbreakertopology the values are always preserved
+            // when using only the busbreakerview. So mimic the behavior and always preserve them
+            // when using the busview: get V and Angle values from configured buses
+            String configuredBusId = ((ConnectedSetResult<String>) connectedSet).getConnectedNodesOrBuses().iterator().next(); // nondeterministic
+            Bus b = index.getConfiguredBus(configuredBusId).orElseThrow();
+            v = b.getV();
+            angle = b.getAngle();
+        }
+        return new CalculatedBusAttributes(connectedSet.getConnectedVertices(), null, null, v, angle);


This if/else makes me think that this should be implemented in BusBreakerTopology/NodeBreakerTopology classes ? Maybe an abstraction here

antoinebhs · 2024-12-05T08:04:44Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CalculatedBus.java

@@ -471,4 +509,43 @@ public <E extends Extension<Bus>, B extends ExtensionAdder<Bus, E>> B newExtensi
    public int getCalculatedBusNum() {
        return calculatedBusNum;
    }
+
+    private void updateBusesAttributes(double value, ObjDoubleConsumer<CalculatedBusAttributes> setValue) {
+        // busnum of this bus -> nodes in this bus -> busnums in the other view -> buses of the other view


This comment is not clear

antoinebhs · 2024-12-05T08:08:50Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CalculatedBus.java

+                        List<CalculatedBusAttributes> calculatedBusAttributes = isBusView
+                            ? vlAttributes.getCalculatedBusesForBusBreakerView()
+                            : vlAttributes.getCalculatedBusesForBusView();


Why is this inside the for loop? It looks like it's constant? Similarly to nodesToCalculatedBuses, etc

antoinebhs · 2024-12-05T08:18:11Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CalculatedBus.java

+    private void setVInCalculatedBus(CalculatedBusAttributes calculatedBusAttributes, double value) {
+        calculatedBusAttributes.setV(value);
+    }
+
+    private void setVInConfiguredBus(ConfiguredBusImpl configuredBus, double value) {
+        configuredBus.setConfiguredBusV(value);
+    }
+
+    private void setAngleInCalculatedBus(CalculatedBusAttributes calculatedBusAttributes, double value) {
+        calculatedBusAttributes.setAngle(value);
+    }
+
+    private void setAngleInConfiguredBus(ConfiguredBusImpl configuredBus, double value) {
+        configuredBus.setConfiguredBusAngle(value);
+    }


Can we have some consistency in naming?
setAngleInConfiguredBus // setConfiguredBusAngle
setAngleInCalculatedBus // setAngle
same for V

antoinebhs · 2024-12-05T08:21:25Z

...ork-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/ConfiguredBusImpl.java

+        return this;
+    }
+
+    void setAngle(double angle, boolean updateCalculatedBus) {


antoinebhs · 2024-12-05T08:24:21Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+            .add();
+
+        // add busbar sections in star coupling to have a bus in the middle without any equipment. It will
+        // exist in the busbreaker view but not in the busview. For this one use closed switches so that it


Lot of suspense...

antoinebhs · 2024-12-05T08:27:25Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+/**
+ * @author Jon Harper <jon.harper at rte-france.com>
+ *
+ * complex specific exhaustive tests for setV and setAngle interactions with calculated views.


FranckLecuyer · 2024-12-10T12:34:39Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/AbstractTopology.java

+            // currently for busbreakertopology the values are always preserved
+            // when using only the busbreakerview. So mimic the behavior and always preserve them
+            // when using the busview: get V and Angle values from configured buses
+            String configuredBusId = ((ConnectedSetResult<String>) connectedSet).getConnectedNodesOrBuses().iterator().next(); // nondeterministic


Maybe check iterator with hasNext before calling next ...

FranckLecuyer · 2024-12-10T12:39:26Z

network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/AbstractTopology.java

+            // when using only the busbreakerview. So mimic the behavior and always preserve them
+            // when using the busview: get V and Angle values from configured buses
+            String configuredBusId = ((ConnectedSetResult<String>) connectedSet).getConnectedNodesOrBuses().iterator().next(); // nondeterministic
+            Bus b = index.getConfiguredBus(configuredBusId).orElseThrow();


Maybe customize the exception, in order to have a more accurate message ...

FranckLecuyer · 2024-12-10T15:48:52Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+        for (Map.Entry<String, String > entry : busBreakerViewBusToBusViewBus.entrySet()) {
+            String busbreakerviewbusid = entry.getKey();
+            String busviewbusid = entry.getValue();
+            setAllBbvbAndBvb(vl, Double.NaN, setter);


setAllBusBreakerViewBusAndBusViewBus ?

FranckLecuyer · 2024-12-10T15:50:51Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+        if (vl.getTopologyKind() == TopologyKind.NODE_BREAKER) {
+            // test one same view
+            vl = networkVoltageLevelSupplier.get();
+            setAllBbvb(vl, 1.0, setter);


setAllBusBreakerViewBus ?

FranckLecuyer · 2024-12-10T15:51:11Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+
+            // test one other view
+            vl = networkVoltageLevelSupplier.get();
+            setAllBvb(vl, 1.0, setter);


setAllBusViewBus ?

FranckLecuyer · 2024-12-10T15:51:29Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+            // so test only with busview view
+            // NOTE: for busbreakertopology we keep the previous values.
+            vl = networkVoltageLevelSupplier.get();
+            setAllBvb(vl, 1.0, setter);


setAllBusViewBus ?

FranckLecuyer · 2024-12-10T15:52:19Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+        for (Map.Entry<String, List<String> > entry : busViewBusToBusBreakerViewBus.entrySet()) {
+            String busviewbusid = entry.getKey();
+            List<String> busbreakerviewbusids = entry.getValue();
+            setAllBbvbAndBvb(vl, Double.NaN, setter);


setAllBusBreakerViewBusAndBusViewBus ?

FranckLecuyer · 2024-12-10T15:52:53Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+        // test one same view, nodebreakertopology and busbreakertopology behave the same here
+        if (vl.getTopologyKind() == TopologyKind.NODE_BREAKER) {
+            vl = networkVoltageLevelSupplier.get();
+            setAllBvb(vl, 1.0, setter);


setAllBusViewBus ?

FranckLecuyer · 2024-12-10T15:53:09Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+
+            // test one other view
+            vl = networkVoltageLevelSupplier.get();
+            setAllBvb(vl, 1.0, setter);


setAllBusViewBus ?

FranckLecuyer · 2024-12-10T15:53:23Z

...est/java/com/powsybl/network/store/iidm/impl/VoltageLevelSetVAngleInCalculatedViewsTest.java

+            // so test only with busview view
+            // NOTE: for busbreakertopology we keep the previous values.
+            vl = networkVoltageLevelSupplier.get();
+            setAllBvb(vl, 1.0, setter);


setAllBusViewBus ?

FranckLecuyer and others added 9 commits September 16, 2024 09:41

Synchronization between views for v and angle bus values.

a43f002

Signed-off-by: Franck LECUYER <[email protected]>

Merge branch 'main' into synchronize_set_v_angle_on_bus_between_views

8492ef9

Merge branch 'main' into synchronize_set_v_angle_on_bus_between_views

3afafdd

Merge branch 'main' into synchronize_set_v_angle_on_bus_between_views

9ce5cb7

Changes after review

6661c1f

Add new test with multiple buses in bus view and bus breaker view in a voltage level Signed-off-by: Franck LECUYER <[email protected]>

Fix sonar issue

3b26409

Signed-off-by: Franck LECUYER <[email protected]>

Merge branch 'main' into synchronize_set_v_angle_on_bus_between_views

98bf508

more exhaustive test

4c2dffb

Signed-off-by: HARPER Jon <[email protected]>

wip fix syncv

d09854e

Signed-off-by: HARPER Jon <[email protected]>

jonenst requested a review from FranckLecuyer September 27, 2024 16:14

more fixes using busnum instead of vertex

ed42836

Signed-off-by: HARPER Jon <[email protected]>

FranckLecuyer added 4 commits September 30, 2024 17:32

Fix updateBusesAttributes method in CalculatedBus.java

54ef522

Signed-off-by: Franck LECUYER <[email protected]>

Cleaning and optimizing code

d59aada

Signed-off-by: Franck LECUYER <[email protected]>

Move methods in another file

ebe387e

Signed-off-by: Franck LECUYER <[email protected]>

Add configuration similar to the one in AbstractMainConnectedComponen…

16edded

…tWithSwitchTest, with buses in bus breaker view between retained switches and with no equipments. Signed-off-by: Franck LECUYER <[email protected]>

FranckLecuyer force-pushed the fixsyncv branch from 0d58627 to 16edded Compare October 3, 2024 14:04

Clean code in getVAndAngleFromOtherView method

15df0fa

Signed-off-by: Franck LECUYER <[email protected]>

FranckLecuyer requested a review from SlimaneAmar October 4, 2024 08:13

FranckLecuyer changed the title ~~wip fix syncv~~ fix syncv Oct 22, 2024

jonenst commented Oct 22, 2024

View reviewed changes

jonenst changed the base branch from synchronize_set_v_angle_on_bus_between_views to main October 22, 2024 09:00

jonenst commented Oct 22, 2024

View reviewed changes

VoltageLevelTest, Add more combinations to setV/setAngle for cache ex…

6853516

…istence Signed-off-by: HARPER Jon <[email protected]>

jonenst requested a review from antoinebhs December 4, 2024 23:12

Improve exhaustive test, make nodebreaker consistent with invalid vie…

9c17fe0

…ws, simplify code, throw exceptions instead of silently doing nothing Signed-off-by: HARPER Jon <[email protected]>

jonenst force-pushed the fixsyncv branch from a984abb to 9c17fe0 Compare December 4, 2024 23:21

antoinebhs reviewed Dec 5, 2024

View reviewed changes

FranckLecuyer requested changes Dec 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix syncv #460

fix syncv #460

jonenst commented Sep 27, 2024

jonenst commented Sep 30, 2024

jonenst Oct 22, 2024

jonenst Oct 22, 2024 •

edited

Loading

jonenst Oct 22, 2024

jonenst Oct 22, 2024

sonarcloud bot commented Oct 22, 2024

jonenst commented Dec 4, 2024

sonarcloud bot commented Dec 4, 2024

antoinebhs left a comment

antoinebhs Dec 5, 2024

antoinebhs Dec 5, 2024

antoinebhs Dec 5, 2024

antoinebhs Dec 5, 2024

antoinebhs Dec 5, 2024

antoinebhs Dec 5, 2024

antoinebhs Dec 5, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

FranckLecuyer Dec 10, 2024

fix syncv #460

Are you sure you want to change the base?

fix syncv #460

Conversation

jonenst commented Sep 27, 2024

jonenst commented Sep 30, 2024

Choose a reason for hiding this comment

jonenst Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 22, 2024

Quality Gate passed

jonenst commented Dec 4, 2024

sonarcloud bot commented Dec 4, 2024

Quality Gate passed

antoinebhs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonenst Oct 22, 2024 •

edited

Loading