Fix implementation and add TopologyTraverserTest

Signed-off-by: Sébastien LAIGRE <[email protected]>
powsybl · Mar 14, 2022 · eb5d179 · eb5d179
1 parent c46cddb
commit eb5d179
Show file tree

Hide file tree

Showing 15 changed files with 476 additions and 100 deletions.
diff --git a/include/powsybl/iidm/Terminal.hpp b/include/powsybl/iidm/Terminal.hpp
@@ -88,7 +88,7 @@ class Terminal : public MultiVariantObject {
 
     virtual void traverse(voltage_level::TopologyTraverser& traverser) = 0;
 
-    virtual void traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) = 0;
+    virtual bool traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) = 0;
 
 protected: // MultiVariantObject
     void allocateVariantArrayElement(const std::set<unsigned long>& indexes, unsigned long sourceIndex) override;

diff --git a/include/powsybl/iidm/VoltageLevelTopologyTraverser.hpp b/include/powsybl/iidm/VoltageLevelTopologyTraverser.hpp
@@ -8,6 +8,8 @@
 #ifndef POWSYBL_IIDM_VOLTAGELEVELTOPOLOGYTRAVERSER_HPP
 #define POWSYBL_IIDM_VOLTAGELEVELTOPOLOGYTRAVERSER_HPP
 
+#include <powsybl/math/TraverseResult.hpp>
+
 namespace powsybl {
 
 namespace iidm {
@@ -31,9 +33,9 @@ class TopologyTraverser {
 
     TopologyTraverser& operator=(TopologyTraverser&&) noexcept = default;
 
-    virtual bool traverse(Terminal& terminal, bool connected) = 0;
+    virtual math::TraverseResult traverse(Terminal& terminal, bool connected) = 0;
 
-    virtual bool traverse(Switch& aSwitch) = 0;
+    virtual math::TraverseResult traverse(Switch& aSwitch) = 0;
 };
 
 }  // namespace voltage_level

diff --git a/src/iidm/BusBreakerVoltageLevel.cpp b/src/iidm/BusBreakerVoltageLevel.cpp
@@ -266,6 +266,11 @@ const TopologyKind& BusBreakerVoltageLevel::getTopologyKind() const {
     return s_topologyKind;
 }
 
+math::TraverseResult BusBreakerVoltageLevel::getTraverserResult(TerminalSet& visitedTerminals, BusTerminal& terminal, TopologyTraverser& traverser) {
+    auto pair = visitedTerminals.insert(terminal);
+    return pair.second ? traverser.traverse(terminal, true) : math::TraverseResult::TERMINATE_PATH;
+}
+
 stdcxx::optional<unsigned long> BusBreakerVoltageLevel::getVertex(const std::string& busId, bool throwException) const {
     checkNotEmpty(busId, "bus id is null");
 
@@ -366,52 +371,57 @@ void BusBreakerVoltageLevel::traverse(BusTerminal& terminal, VoltageLevel::Topol
     traverse(terminal, traverser, traversedTerminals);
 }
 
-void BusBreakerVoltageLevel::traverse(BusTerminal& terminal, VoltageLevel::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const {
-    if (traversedTerminals.find(terminal) != traversedTerminals.end()) {
-        return;
-    }
-
-    TerminalSet nextTerminals;
-
+bool BusBreakerVoltageLevel::traverse(BusTerminal& terminal, VoltageLevel::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const {
     // check if we are allowed to traverse the terminal itself
-    if (traverser.traverse(terminal, terminal.isConnected())) {
-        traversedTerminals.emplace(terminal);
+    math::TraverseResult termTraverseResult = getTraverserResult(traversedTerminals, terminal, traverser);
+    if (termTraverseResult == math::TraverseResult::TERMINATE_TRAVERSER) {
+        return false;
+    }
 
+    if (termTraverseResult == math::TraverseResult::CONTINUE) {
+        TerminalSet nextTerminals;
         addNextTerminals(terminal, nextTerminals);
 
-        // then check we can traverse terminal connected to same bus
+        // then check we can traverse terminals connected to same bus
         unsigned long v = *getVertex(terminal.getConnectableBusId(), true);
-        ConfiguredBus& bus = m_graph.getVertexObject(v);
-        for (Terminal& t : bus.getTerminals()) {
-            if (!stdcxx::areSame(t, terminal) && traverser.traverse(t, t.isConnected())) {
+        ConfiguredBus& bus = m_graph.getVertexObject(v).get();
+        for (BusTerminal& t : bus.getTerminals()) {
+            math::TraverseResult tTraverseResult = getTraverserResult(traversedTerminals, t, traverser);
+            if (tTraverseResult == math::TraverseResult::TERMINATE_TRAVERSER) {
+                return false;
+            }
+            if (tTraverseResult == math::TraverseResult::CONTINUE) {
                 addNextTerminals(t, nextTerminals);
             }
         }
 
         // then go through other buses of the voltage level
-        m_graph.traverse(v, [this, &traverser, &traversedTerminals, &nextTerminals](unsigned long /*v1*/, unsigned long e, unsigned long v2) {
+        bool traversalTerminated = !m_graph.traverse(v, [this, &nextTerminals, &traverser, &traversedTerminals](unsigned long /*v1*/, unsigned long e, unsigned long v2) {
             Switch& aSwitch = m_graph.getEdgeObject(e);
-            ConfiguredBus& otherBus = m_graph.getVertexObject(v2);
-            if (traverser.traverse(aSwitch)) {
-                if (otherBus.getTerminalCount() == 0) {
-                    return math::TraverseResult::CONTINUE;
-                }
-
-                BusTerminal& otherTerminal = *otherBus.getTerminals().begin();
-                if (traverser.traverse(otherTerminal, otherTerminal.isConnected())) {
-                    traversedTerminals.emplace(otherTerminal);
-
+            const stdcxx::range<BusTerminal>& otherBusTerminals = m_graph.getVertexObject(v2).get().getTerminals();
+            math::TraverseResult switchTraverseResult = traverser.traverse(aSwitch);
+            if (switchTraverseResult == math::TraverseResult::CONTINUE && !otherBusTerminals.empty()) {
+                BusTerminal& otherTerminal = *otherBusTerminals.begin();
+                math::TraverseResult otherTermTraverseResult = getTraverserResult(traversedTerminals, otherTerminal, traverser);
+                if (otherTermTraverseResult == math::TraverseResult::CONTINUE) {
                     addNextTerminals(otherTerminal, nextTerminals);
-                    return math::TraverseResult::CONTINUE;
                 }
+                return otherTermTraverseResult;
             }
-            return math::TraverseResult::TERMINATE_PATH;
+            return switchTraverseResult;
         });
+        if (traversalTerminated) {
+            return false;
+        }
 
-        for (Terminal& t : nextTerminals) {
-            t.traverse(traverser, traversedTerminals);
+        for (Terminal& terminal : nextTerminals) {
+            if (!terminal.traverse(traverser, traversedTerminals)) {
+                return false;
+            }
         }
     }
+
+    return true;
 }
 
 }  // namespace iidm

diff --git a/src/iidm/BusBreakerVoltageLevel.hpp b/src/iidm/BusBreakerVoltageLevel.hpp
@@ -68,7 +68,7 @@ class BusBreakerVoltageLevel : public VoltageLevel {
 
     void traverse(BusTerminal& terminal, voltage_level::TopologyTraverser& traverser) const;
 
-    void traverse(BusTerminal& terminal, voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const;
+    bool traverse(BusTerminal& terminal, voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const;
 
 protected: // MultiVariantObject
     void allocateVariantArrayElement(const std::set<unsigned long>& indexes, unsigned long sourceIndex) override;
@@ -91,6 +91,9 @@ class BusBreakerVoltageLevel : public VoltageLevel {
 
     void removeTopology() override;
 
+private:
+    static math::TraverseResult getTraverserResult(TerminalSet& visitedTerminals, BusTerminal& terminal, TopologyTraverser& traverser);
+
 private:
     void checkTerminal(Terminal& terminal) const;
 

diff --git a/src/iidm/BusTerminal.cpp b/src/iidm/BusTerminal.cpp
@@ -116,11 +116,10 @@ void BusTerminal::traverse(voltage_level::TopologyTraverser& traverser) {
     dynamic_cast<BusBreakerVoltageLevel&>(getVoltageLevel()).traverse(*this, traverser);
 }
 
-void BusTerminal::traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) {
-    dynamic_cast<BusBreakerVoltageLevel&>(getVoltageLevel()).traverse(*this, traverser, traversedTerminals);
+bool BusTerminal::traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) {
+    return dynamic_cast<BusBreakerVoltageLevel&>(getVoltageLevel()).traverse(*this, traverser, traversedTerminals);
 }
 
-
 std::ostream& operator<<(std::ostream& stream, const BusTerminal& busTerminal) {
     stream << stdcxx::simpleClassName(busTerminal) << "[" << busTerminal.getConnectableBusId() << "]";
 

diff --git a/src/iidm/BusTerminal.hpp b/src/iidm/BusTerminal.hpp
@@ -50,7 +50,7 @@ class BusTerminal : public Terminal {
 
     void traverse(voltage_level::TopologyTraverser& traverser) override;
 
-    void traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) override;
+    bool traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) override;
 
 public:
     BusTerminal(VoltageLevel& voltageLevel, const std::string& connectableBusId, bool connected);

diff --git a/src/iidm/NodeBreakerVoltageLevel.cpp b/src/iidm/NodeBreakerVoltageLevel.cpp
@@ -333,6 +333,11 @@ const TopologyKind& NodeBreakerVoltageLevel::getTopologyKind() const {
     return s_topologyKind;
 }
 
+math::TraverseResult NodeBreakerVoltageLevel::getTraverseResult(TerminalSet& visitedTerminals, NodeTerminal& terminal, TopologyTraverser& traverser) {
+    auto pair = visitedTerminals.insert(terminal);
+    return pair.second ? traverser.traverse(terminal, true) : math::TraverseResult::TERMINATE_PATH;
+}
+
 void NodeBreakerVoltageLevel::invalidateCache() {
     m_variants.get().getCalculatedBusTopology().invalidateCache();
     m_variants.get().getCalculatedBusBreakerTopology().invalidateCache();
@@ -395,42 +400,42 @@ void NodeBreakerVoltageLevel::traverse(NodeTerminal& terminal, VoltageLevel::Top
     traverse(terminal, traverser, traversedTerminals);
 }
 
-void NodeBreakerVoltageLevel::traverse(NodeTerminal& terminal, VoltageLevel::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const {
-    if (traversedTerminals.find(terminal) != traversedTerminals.end()) {
-        return;
+bool NodeBreakerVoltageLevel::traverse(NodeTerminal& terminal, VoltageLevel::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const {
+    const math::TraverseResult& termTraverseResult = getTraverseResult(traversedTerminals, terminal, traverser);
+    if (termTraverseResult == math::TraverseResult::TERMINATE_TRAVERSER) {
+        return false;
     }
-
-    if (traverser.traverse(terminal, true)) {
-        traversedTerminals.emplace(terminal);
-
-        unsigned long node = terminal.getNode();
+    if (termTraverseResult == math::TraverseResult::CONTINUE) {
         TerminalSet nextTerminals;
-
         addNextTerminals(terminal, nextTerminals);
 
-        m_graph.traverse(node, [this, &traverser, &traversedTerminals, &nextTerminals](unsigned long /*v1*/, unsigned long e, unsigned long v2) {
-            const stdcxx::Reference<Switch>& aSwitch = m_graph.getEdgeObject(e);
-            const stdcxx::Reference<NodeTerminal>& otherTerminal = m_graph.getVertexObject(v2);
-            if (!aSwitch // internal connection case
-                || traverser.traverse(aSwitch)) {
-                if (!otherTerminal) {
-                    return math::TraverseResult::CONTINUE;
-                }
-                if (traverser.traverse(otherTerminal.get(), true)) {
-                    traversedTerminals.emplace(otherTerminal.get());
-
-                    addNextTerminals(otherTerminal.get(), nextTerminals);
-                    return math::TraverseResult::CONTINUE;
+        unsigned long node = terminal.getNode();
+        bool traverseTerminated = !m_graph.traverse(node, [this, &traverser, &traversedTerminals, &nextTerminals](unsigned long /*v1*/, unsigned long e, unsigned long v2) {
+            const auto& aSwitch = m_graph.getEdgeObject(e);
+            const auto& otherTerminal = m_graph.getVertexObject(v2);
+            const math::TraverseResult& edgeTraverseResult = aSwitch ? traverser.traverse(aSwitch) : math::TraverseResult::CONTINUE; // internal connection case
+            if (edgeTraverseResult == math::TraverseResult::CONTINUE && otherTerminal) {
+                math::TraverseResult otherTermTraverseResult = getTraverseResult(traversedTerminals, otherTerminal, traverser);
+                if (otherTermTraverseResult == math::TraverseResult::CONTINUE) {
+                    addNextTerminals(otherTerminal, nextTerminals);
                 }
-                return math::TraverseResult::TERMINATE_PATH;
+                return otherTermTraverseResult;
             }
-            return math::TraverseResult::TERMINATE_PATH;
+            return edgeTraverseResult;
         });
 
-        for (auto nextTerminal : nextTerminals) {
-            nextTerminal.get().traverse(traverser, traversedTerminals);
+        if (traverseTerminated) {
+            return false;
+        }
+
+        for (Terminal& nextTerminal : nextTerminals) {
+            if (!nextTerminal.traverse(traverser, traversedTerminals)) {
+                return false;
+            }
         }
     }
+
+    return true;
 }
 
 }  // namespace iidm

diff --git a/src/iidm/NodeBreakerVoltageLevel.hpp b/src/iidm/NodeBreakerVoltageLevel.hpp
@@ -66,7 +66,7 @@ class NodeBreakerVoltageLevel : public VoltageLevel {
 
     void traverse(NodeTerminal& terminal, VoltageLevel::TopologyTraverser& traverser) const;
 
-    void traverse(NodeTerminal& terminal, VoltageLevel::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const;
+    bool traverse(NodeTerminal& terminal, VoltageLevel::TopologyTraverser& traverser, TerminalSet& traversedTerminals) const;
 
 public:
     NodeBreakerVoltageLevel(const std::string& id, const std::string& name, bool fictitious, const stdcxx::Reference<Substation>& substation,
@@ -97,6 +97,9 @@ class NodeBreakerVoltageLevel : public VoltageLevel {
 private:  // VoltageLevel
     void removeTopology() override;
 
+private:
+    static math::TraverseResult getTraverseResult(TerminalSet& visitedTerminals, NodeTerminal& terminal, TopologyTraverser& traverser);
+
 private:
     void checkTerminal(Terminal& terminal) const;
 

diff --git a/src/iidm/NodeTerminal.cpp b/src/iidm/NodeTerminal.cpp
@@ -139,8 +139,8 @@ void NodeTerminal::traverse(voltage_level::TopologyTraverser& traverser) {
     dynamic_cast<NodeBreakerVoltageLevel&>(getVoltageLevel()).traverse(*this, traverser);
 }
 
-void NodeTerminal::traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) {
-    dynamic_cast<NodeBreakerVoltageLevel&>(getVoltageLevel()).traverse(*this, traverser, traversedTerminals);
+bool NodeTerminal::traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) {
+    return dynamic_cast<NodeBreakerVoltageLevel&>(getVoltageLevel()).traverse(*this, traverser, traversedTerminals);
 }
 
 }  // namespace iidm

diff --git a/src/iidm/NodeTerminal.hpp b/src/iidm/NodeTerminal.hpp
@@ -48,7 +48,7 @@ class NodeTerminal : public Terminal {
 
     void traverse(voltage_level::TopologyTraverser& traverser) override;
 
-    void traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) override;
+    bool traverse(voltage_level::TopologyTraverser& traverser, TerminalSet& traversedTerminals) override;
 
 public:
     NodeTerminal(VoltageLevel& voltageLevel, unsigned long node);

diff --git a/test/iidm/CMakeLists.txt b/test/iidm/CMakeLists.txt
@@ -42,6 +42,7 @@ set(UNIT_TEST_SOURCES
     SubstationTest.cpp
     TerminalTest.cpp
     TopologyLevelTest.cpp
+    TopologyTraverserTest.cpp
     ThreeWindingsTransformerTest.cpp
     TieLineTest.cpp
     TwoWindingsTransformerTest.cpp