Fix traverser in CalculatedBusTopology (#3242)

* Add failing unit test with loop * Fix duplication in nodes TIntArrayList * Add clarifying comment Signed-off-by: Florian Dupuy <[email protected]>
powsybl · Dec 4, 2024 · 87eeec5 · 87eeec5
1 parent f235935
commit 87eeec5
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 4 deletions.
diff --git a/iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/NodeBreakerTopologyModel.java b/iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/NodeBreakerTopologyModel.java
@@ -263,15 +263,20 @@ private void traverse(int n, boolean[] encountered, Predicate<SwitchImpl> termin
             if (!encountered[n]) {
                 final TIntArrayList nodes = new TIntArrayList(1);
                 nodes.add(n);
-                graph.traverse(n, TraversalType.DEPTH_FIRST, (n1, e, n2) -> {
+                Traverser traverser = (n1, e, n2) -> {
                     SwitchImpl aSwitch = graph.getEdgeObject(e);
                     if (aSwitch != null && terminate.test(aSwitch)) {
                         return TraverseResult.TERMINATE_PATH;
                     }
 
-                    nodes.add(n2);
+                    if (!encountered[n2]) {
+                        // We need to check this as the traverser might be called twice with the same n2 but with different edges.
+                        // Note that the "encountered" array is used and maintained inside graph::traverse method, hence we should not update it.
+                        nodes.add(n2);
+                    }
                     return TraverseResult.CONTINUE;
-                }, encountered);
+                };
+                graph.traverse(n, TraversalType.DEPTH_FIRST, traverser, encountered);
 
                 // check that the component is a bus
                 String busId = Identifiables.getUniqueId(NAMING_STRATEGY.getId(voltageLevel, nodes), getNetwork().getIndex()::contains);

diff --git a/iidm/iidm-tck/src/test/java/com/powsybl/iidm/network/tck/AbstractNodeBreakerTest.java b/iidm/iidm-tck/src/test/java/com/powsybl/iidm/network/tck/AbstractNodeBreakerTest.java
@@ -312,6 +312,33 @@ private static Network createIsolatedLoadNetwork() {
         return network;
     }
 
+    /**
+     * <pre>
+     *                load
+     *                  |
+     *               ___|___
+     *               |     |
+     *           fd1 x     x fd2
+     * bbs1 _________|__   |
+     *        |            |
+     *        c            |
+     * bbs2 __|____________|__
+     * </pre>
+     */
+    private static Network createNetworkWithLoop() {
+        Network network = Network.create("test", "test");
+        Substation substation = network.newSubstation().setId("s").add();
+        VoltageLevel vl = substation.newVoltageLevel().setId("vl").setNominalV(400).setTopologyKind(TopologyKind.NODE_BREAKER).add();
+        VoltageLevel.NodeBreakerView topology = vl.getNodeBreakerView();
+        topology.newBusbarSection().setId("bbs1").setNode(0).add();
+        topology.newBusbarSection().setId("bbs2").setNode(1).add();
+        topology.newDisconnector().setId("fd1").setNode1(0).setNode2(2).add();
+        topology.newDisconnector().setId("fd2").setNode1(1).setNode2(2).add();
+        topology.newBreaker().setId("c").setNode1(0).setNode2(1).add();
+        vl.newLoad().setId("load").setNode(2).setP0(10).setQ0(3).add();
+        return network;
+    }
+
     @Test
     public void connectDisconnectRemove() {
         Network network = createNetwork();
@@ -533,4 +560,19 @@ public void testRemove() {
         sub.remove();
         assertNull(network.getSubstation("S1"));
     }
+
+    @Test
+    public void testCalculatedBusTopologyWithLoop() {
+        Network n = createNetworkWithLoop();
+
+        Bus busBbv = n.getBusBreakerView().getBus("vl_0");
+        assertNotNull(busBbv);
+        assertEquals(1, n.getBusBreakerView().getBusCount());
+        assertEquals(3, busBbv.getConnectedTerminalCount());
+
+        Bus busBv = n.getBusView().getBus("vl_0");
+        assertNotNull(busBv);
+        assertEquals(1, n.getBusView().getBusStream().count());
+        assertEquals(3, busBv.getConnectedTerminalCount());
+    }
 }
diff --git a/math/src/main/java/com/powsybl/math/graph/UndirectedGraph.java b/math/src/main/java/com/powsybl/math/graph/UndirectedGraph.java
@@ -321,7 +321,11 @@ public interface UndirectedGraph<V, E> {
      * @param v the vertex index where the traverse has to start.
      * @param traversalType the type of traversal (breadth-first or depth-first)
      * @param traverser the {@link Traverser} instance to use to know if the traverse should continue or stop.
-     * @param verticesEncountered the list of traversed vertices.
+     * @param verticesEncountered the list of traversed vertices - a vertex is considered as traversed:
+     *                           <ul>
+     *                            <li>if it is the starting vertex</li>
+     *                            <li>if one of its edges has been traversed, with a traverser result {@link TraverseResult#CONTINUE}</li>
+     *                           </ul>
      * @return false if the whole traversing has to stop, meaning that a {@link TraverseResult#TERMINATE_TRAVERSER}
      * has been returned from the traverser, true otherwise
      */