Fix bus/breaker LF network and non impedant branches (#321)

* Add a boolean to say if a branch is part of the minimal spanning tree.
* Merge voltage controls / discrete voltage controls on non-impedant connected set
* Add unit test

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Co-authored-by: Hadrien <hadrien.godard@artelys.com>
Co-authored-by: Anne Tilloy <anne.tilloy@rte-france.com>
Co-authored-by: Florian Dupuy <florian.dupuy@rte-france.com>

src/main/java/com/powsybl/openloadflow/ac/equations/AcEquationSystem.java

@@ -11,14 +11,13 @@
 import com.powsybl.openloadflow.equations.*;
 import com.powsybl.openloadflow.network.*;
 import com.powsybl.openloadflow.network.DiscretePhaseControl.Mode;
-import org.jgrapht.Graph;
-import org.jgrapht.alg.connectivity.ConnectivityInspector;
-import org.jgrapht.alg.interfaces.SpanningTreeAlgorithm;
-import org.jgrapht.alg.spanning.KruskalMinimumSpanningTree;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 /**
@@ -117,6 +116,9 @@ private static List<EquationTerm> createReactiveTerms(LfBus controllerBus, Varia
         for (LfBranch branch : controllerBus.getBranches()) {
             EquationTerm q;
             if (LfNetwork.isZeroImpedanceBranch(branch)) {
+                if (!branch.isSpanningTreeEdge()) {
+                    continue;
+                }
                 if (branch.getBus1() == controllerBus) {
                     q = EquationTerm.createVariableTerm(branch, VariableType.DUMMY_Q, variableSet);
                 } else {
@@ -426,23 +428,10 @@ private static void createBranchEquations(LfNetwork network, VariableSet variabl
             .filter(b -> !LfNetwork.isZeroImpedanceBranch(b))
             .forEach(b -> createImpedantBranch(b, b.getBus1(), b.getBus2(), variableSet, creationParameters, equationSystem));
 
-        // create zero impedance equations only on minimum spanning forest calculated from zero impedance sub graph
-        Graph<LfBus, LfBranch> zeroImpedanceSubGraph = network.createZeroImpedanceSubGraph();
-        if (!zeroImpedanceSubGraph.vertexSet().isEmpty()) {
-            List<Set<LfBus>> connectedSets = new ConnectivityInspector<>(zeroImpedanceSubGraph).connectedSets();
-            for (Set<LfBus> connectedSet : connectedSets) {
-                if (connectedSet.size() > 2 && connectedSet.stream().filter(LfBus::isVoltageControllerEnabled).count() > 1) {
-                    String problBuses = connectedSet.stream().map(LfBus::getId).collect(Collectors.joining(", "));
-                    throw new PowsyblException(
-                        "Zero impedance branches that connect at least two buses with voltage control (buses: " + problBuses + ")");
-                }
-            }
-
-            SpanningTreeAlgorithm.SpanningTree<LfBranch> spanningTree = new KruskalMinimumSpanningTree<>(zeroImpedanceSubGraph).getSpanningTree();
-            for (LfBranch branch : spanningTree.getEdges()) {
-                createNonImpedantBranch(variableSet, equationSystem, branch, branch.getBus1(), branch.getBus2());
-            }
-        }
+        // create zero and non zero impedance branch equations
+        network.getBranches().stream()
+                .filter(b -> LfNetwork.isZeroImpedanceBranch(b) && b.isSpanningTreeEdge())
+                .forEach(b -> createNonImpedantBranch(variableSet, equationSystem, b, b.getBus1(), b.getBus2()));
     }
 
     public static EquationSystem create(LfNetwork network) {

src/main/java/com/powsybl/openloadflow/network/AbstractLfBranch.java

@@ -68,6 +68,8 @@ public void setAcceptableDuration(int acceptableDuration) {
 
     protected boolean disabled = false;
 
+    protected boolean spanningTreeEdge = false;
+
     protected AbstractLfBranch(LfNetwork network, LfBus bus1, LfBus bus2, PiModel piModel) {
         super(network);
         this.bus1 = bus1;
@@ -236,4 +238,14 @@ public double computeApparentPower2() {
         double q = getQ2().eval();
         return FastMath.sqrt(p * p + q * q);
     }
+
+    @Override
+    public void setSpanningTreeEdge(boolean spanningTreeEdge) {
+        this.spanningTreeEdge = spanningTreeEdge;
+    }
+
+    @Override
+    public boolean isSpanningTreeEdge() {
+        return this.spanningTreeEdge;
+    }
 }

src/main/java/com/powsybl/openloadflow/network/LfBranch.java

@@ -81,4 +81,8 @@ public interface LfBranch extends LfElement {
     double computeApparentPower1();
 
     double computeApparentPower2();
+
+    void setSpanningTreeEdge(boolean spanningTreeEdge);
+
+    boolean isSpanningTreeEdge();
 }

src/main/java/com/powsybl/openloadflow/network/LfBus.java

@@ -51,6 +51,8 @@ public interface LfBus extends LfElement {
 
     Optional<VoltageControl> getVoltageControl();
 
+    void removeVoltageControl();
+
     void setVoltageControl(VoltageControl voltageControl);
 
     double getTargetP();

src/main/java/com/powsybl/openloadflow/network/impl/AbstractLfBus.java

@@ -117,6 +117,10 @@ public Optional<VoltageControl> getVoltageControl() {
         return Optional.ofNullable(voltageControl);
     }
 
+    public void removeVoltageControl() {
+        this.voltageControl = null;
+    }
+
     @Override
     public void setVoltageControl(VoltageControl voltageControl) {
         Objects.requireNonNull(voltageControl);

src/main/java/com/powsybl/openloadflow/network/impl/LfNetworkLoaderImpl.java

@@ -15,7 +15,10 @@
 import com.powsybl.openloadflow.network.*;
 import net.jafama.FastMath;
 import org.apache.commons.lang3.tuple.Pair;
+import org.jgrapht.Graph;
 import org.jgrapht.alg.connectivity.ConnectivityInspector;
+import org.jgrapht.alg.interfaces.SpanningTreeAlgorithm;
+import org.jgrapht.alg.spanning.KruskalMinimumSpanningTree;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -343,53 +346,89 @@ private static void createSwitches(List<Switch> switches, LfNetwork lfNetwork) {
         }
     }
 
-    private static void fixDiscreteVoltageControls(LfNetwork lfNetwork, boolean minImpedance) {
+    private static void fixAllVoltageControls(LfNetwork lfNetwork, boolean minImpedance, boolean transformerVoltageControl) {
         // If min impedance is set, there is no zero-impedance branch
         if (!minImpedance) {
             // Merge the discrete voltage control in each zero impedance connected set
             List<Set<LfBus>> connectedSets = new ConnectivityInspector<>(lfNetwork.createZeroImpedanceSubGraph()).connectedSets();
-            connectedSets.forEach(LfNetworkLoaderImpl::fixZeroImpendanceNetworkWithSeveralControlledBuses);
+            connectedSets.forEach(set -> mergeVoltageControls(set, transformerVoltageControl));
         }
     }
 
-    private static void fixZeroImpendanceNetworkWithSeveralControlledBuses(Set<LfBus> zeroImpedanceConnectedSet) {
+    private static void mergeVoltageControls(Set<LfBus> zeroImpedanceConnectedSet, boolean transformerVoltageControl) {
+        // Get the list of voltage controls from controlled buses in the zero impedance connected set
+        List<VoltageControl> voltageControls = zeroImpedanceConnectedSet.stream().filter(LfBus::isVoltageControlled)
+                .map(LfBus::getVoltageControl).filter(Optional::isPresent).map(Optional::get)
+                .collect(Collectors.toList());
+
         // Get the list of discrete voltage controls from controlled buses in the zero impedance connected set
-        List<DiscreteVoltageControl> voltageControls = zeroImpedanceConnectedSet.stream().filter(LfBus::isDiscreteVoltageControlled)
-            .map(LfBus::getDiscreteVoltageControl).filter(Optional::isPresent).map(Optional::get)
-            .collect(Collectors.toList());
-        if (voltageControls.isEmpty()) {
+        List<DiscreteVoltageControl> discreteVoltageControls = !transformerVoltageControl ? Collections.emptyList() :
+            zeroImpedanceConnectedSet.stream().filter(LfBus::isDiscreteVoltageControlled)
+                .map(LfBus::getDiscreteVoltageControl).filter(Optional::isPresent).map(Optional::get)
+                .collect(Collectors.toList());
+
+        if (voltageControls.isEmpty() && discreteVoltageControls.isEmpty()) {
             return;
         }
 
-        // The first discrete voltage control is kept and removed from the list
-        DiscreteVoltageControl firstDiscreteVoltageControl = voltageControls.remove(0);
-
-        // First resolve problem of discrete voltage controls
         if (!voltageControls.isEmpty()) {
+            // The first voltage control is kept and removed from the list
+            VoltageControl firstVoltageControl = voltageControls.remove(0);
+
+            // First resolve problem of voltage controls (generator, static var compensator, etc.)
+            // We have several controls whose controlled bus are in the same non-impedant connected set
+            // To solve that we keep only one voltage control (and its target value), the other ones are removed
+            // and the corresponding controllers are added to the control kept
+            LOGGER.info("Zero impedance connected set with several voltage controls: controls are merged");
+            voltageControls.forEach(voltageControl -> checkUniqueTargetV(voltageControl, firstVoltageControl));
+            voltageControls.stream()
+                    .flatMap(vc -> vc.getControllerBuses().stream())
+                    .forEach(controller -> {
+                        firstVoltageControl.addControllerBus(controller);
+                        controller.setVoltageControl(firstVoltageControl);
+                    });
+            voltageControls.stream()
+                    .filter(vc -> !firstVoltageControl.getControllerBuses().contains(vc.getControlledBus()))
+                    .forEach(vc -> vc.getControlledBus().removeVoltageControl());
+
+            // Second, we have to remove all the discrete voltage controls if present.
+            if (!discreteVoltageControls.isEmpty()) {
+                LOGGER.info("Zero impedance connected set with several discrete voltage controls and a voltage control: discrete controls deleted");
+                discreteVoltageControls.stream()
+                        .flatMap(dvc -> dvc.getControllers().stream())
+                        .forEach(controller -> controller.setDiscreteVoltageControl(null));
+                discreteVoltageControls.forEach(dvc -> dvc.getControlled().setDiscreteVoltageControl(null));
+            }
+        } else {
+            // The first discrete voltage control is kept and removed from the list
+            // Note that here we know that there is at least one discrete voltage control
+            DiscreteVoltageControl firstDiscreteVoltageControl = discreteVoltageControls.remove(0);
             // We have several discrete controls whose controlled bus are in the same non-impedant connected set
             // To solve that we keep only one discrete voltage control, the other ones are removed
             // and the corresponding controllers are added to the discrete control kept
             LOGGER.info("Zero impedance connected set with several discrete voltage controls: discrete controls merged");
-            voltageControls.stream()
+            discreteVoltageControls.forEach(dvc -> checkUniqueTargetV(dvc, firstDiscreteVoltageControl));
+            discreteVoltageControls.stream()
                 .flatMap(dvc -> dvc.getControllers().stream())
                 .forEach(controller -> {
                     firstDiscreteVoltageControl.addController(controller);
                     controller.setDiscreteVoltageControl(firstDiscreteVoltageControl);
                 });
-            voltageControls.forEach(dvc -> dvc.getControlled().setDiscreteVoltageControl(null));
+            discreteVoltageControls.forEach(dvc -> dvc.getControlled().setDiscreteVoltageControl(null));
         }
+    }
+
+    private static void checkUniqueTargetV(VoltageControl vc1, VoltageControl vc2) {
+        if (FastMath.abs(vc1.getTargetValue() - vc2.getTargetValue()) > TARGET_V_EPSILON) {
+            LOGGER.error("Two controller buses are controlling buses {} and {} which are in the same non-impedant connected set, but with different target voltages ({} and {}): only target voltage {} is kept",
+                vc1.getControlledBus().getId(), vc2.getControlledBus().getId(), vc1.getTargetValue(), vc2.getTargetValue(), vc2.getTargetValue());
+        }
+    }
 
-        // Then resolve problem of mixed shared controls, that is if there are any generator/svc voltage control together with discrete voltage control(s)
-        // Check if there is one bus with remote voltage control or local voltage control
-        boolean hasControlledBus = zeroImpedanceConnectedSet.stream().anyMatch(LfBus::isVoltageControlled);
-        if (hasControlledBus) {
-            // If any generator/svc voltage controls, remove the merged discrete voltage control
-            // TODO: deal with mixed shared controls instead of removing all discrete voltage controls
-            LOGGER.warn("Zero impedance connected set with voltage control and discrete voltage control: only generator control is kept");
-            // First remove it from controllers
-            firstDiscreteVoltageControl.getControllers().forEach(c -> c.setDiscreteVoltageControl(null));
-            // Then remove it from the controlled lfBus
-            firstDiscreteVoltageControl.getControlled().setDiscreteVoltageControl(null);
+    private static void checkUniqueTargetV(DiscreteVoltageControl dvc1, DiscreteVoltageControl dvc2) {
+        if (FastMath.abs(dvc1.getTargetValue() - dvc2.getTargetValue()) > TARGET_V_EPSILON) {
+            LOGGER.error("Two transformers are controlling buses {} and {} which are in the same non-impedant connected set, but with different target voltages ({} and {}): only target voltage {} is kept",
+                dvc1.getControlled().getId(), dvc2.getControlled().getId(), dvc1.getTargetValue(), dvc2.getTargetValue(), dvc2.getTargetValue());
         }
     }
 
@@ -503,9 +542,18 @@ private static LfNetwork create(int numCC, int numSC, List<Bus> buses, List<Swit
             createSwitches(switches, lfNetwork);
         }
 
-        if (parameters.isTransformerVoltageControl()) {
-            // Fixing discrete voltage controls need to be done after creating switches, as the zero-impedance graph is changed with switches
-            fixDiscreteVoltageControls(lfNetwork, parameters.isMinImpedance());
+        // Fixing voltage controls need to be done after creating switches, as the zero-impedance graph is changed with switches
+        fixAllVoltageControls(lfNetwork, parameters.isMinImpedance(), parameters.isTransformerVoltageControl());
+
+        if (!parameters.isMinImpedance()) {
+            // create zero impedance equations only on minimum spanning forest calculated from zero impedance sub graph
+            Graph<LfBus, LfBranch> zeroImpedanceSubGraph = lfNetwork.createZeroImpedanceSubGraph();
+            if (!zeroImpedanceSubGraph.vertexSet().isEmpty()) {
+                SpanningTreeAlgorithm.SpanningTree<LfBranch> spanningTree = new KruskalMinimumSpanningTree<>(zeroImpedanceSubGraph).getSpanningTree();
+                for (LfBranch branch : spanningTree.getEdges()) {
+                    branch.setSpanningTreeEdge(true);
+                }
+            }
         }
 
         if (report.generatorsDiscardedFromVoltageControlBecauseNotStarted > 0) {

src/test/java/com/powsybl/openloadflow/NonImpedantBranchTest.java

@@ -19,10 +19,9 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import java.util.concurrent.CompletionException;
-
 import static com.powsybl.openloadflow.util.LoadFlowAssert.*;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * @author Geoffroy Jamgotchian <geoffroy.jamgotchian at rte-france.com>
@@ -171,8 +170,9 @@ void inconsistentTargetVoltagesTest() {
         createLine(network, b2, b3, "l23", 0.05);
         createLine(network, b3, b4, "l34", 0.05);
 
-        CompletionException exception = assertThrows(CompletionException.class, () -> loadFlowRunner.run(network, parameters));
-        assertEquals("Non impedant branch 'l12' is connected to PV buses 'b1_vl_0' and 'b2_vl_0' with inconsistent target voltages: 1.0 and 1.01", exception.getCause().getMessage());
+        loadFlowRunner.run(network, parameters);
+
+        assertEquals(b1.getV(), b2.getV(), DELTA_V);
     }
 
     @Test
@@ -227,4 +227,80 @@ void loopNonImpedantBranchTest() {
         result = loadFlowRunner.run(network, parameters);
         assertTrue(result.isOk());
     }
+
+    @Test
+    void twoLinkedPVBusesTest() {
+        Network network = Network.create("TwoPVBusesWithNonImpLine", "code");
+        Bus b1 = createBus(network, "b1");
+        Bus b2 = createBus(network, "b2");
+        createGenerator(b1, "g1", 2, 1);
+        createGenerator(b2, "g2", 2, 1);
+        Line l12 = createLine(network, b1, b2, "l12", 0);
+
+        LoadFlowResult result = loadFlowRunner.run(network, parameters);
+        assertTrue(result.isOk());
+        assertVoltageEquals(1, b1);
+        assertVoltageEquals(1, b2);
+        assertAngleEquals(0, b1);
+        assertAngleEquals(0, b2);
+        assertActivePowerEquals(0, l12.getTerminal1());
+        assertActivePowerEquals(0, l12.getTerminal2());
+        assertReactivePowerEquals(0, l12.getTerminal1());
+        assertReactivePowerEquals(0, l12.getTerminal2());
+    }
+
+    @Test
+    void nonImpedentNetworkWithTwoPVBusesTest() {
+        Network network = Network.create("TwoPVBusesInNonImpNet", "code");
+        Bus b1 = createBus(network, "b1");
+        Bus b2 = createBus(network, "b2");
+        Bus b3 = createBus(network, "b3");
+        createGenerator(b1, "g1", 2, 1);
+        createGenerator(b3, "g3", 2, 1);
+        createLoad(b2, "l2", 4, 2);
+        Line l12 = createLine(network, b1, b2, "l12", 0);
+        Line l23 = createLine(network, b2, b3, "l23", 0);
+
+        LoadFlowResult result = loadFlowRunner.run(network, parameters);
+
+        assertTrue(result.isOk());
+        assertVoltageEquals(1, b1);
+        assertVoltageEquals(1, b2);
+        assertVoltageEquals(1, b3);
+        assertAngleEquals(0, b1);
+        assertAngleEquals(0, b2);
+        assertAngleEquals(0, b3);
+        assertActivePowerEquals(2, l12.getTerminal1());
+        assertActivePowerEquals(-2, l12.getTerminal2());
+        assertActivePowerEquals(-2, l23.getTerminal1());
+        assertActivePowerEquals(2, l23.getTerminal2());
+        assertReactivePowerEquals(1, l12.getTerminal1());
+        assertReactivePowerEquals(-1, l12.getTerminal2());
+        assertReactivePowerEquals(-1, l23.getTerminal1());
+        assertReactivePowerEquals(1, l23.getTerminal2());
+    }
+
+    @Test
+    void nonImpedentNetworkWithCycleTest() {
+        Network network = Network.create("ThreeBusesNetworkWithCycle", "code");
+        Bus b1 = createBus(network, "b1");
+        Bus b2 = createBus(network, "b2");
+        Bus b3 = createBus(network, "b3");
+        createGenerator(b1, "g1", 2, 1);
+        createGenerator(b3, "g3", 2, 1);
+        createLoad(b2, "l2", 4, 2);
+        createLine(network, b1, b2, "l12", 0);
+        createLine(network, b2, b3, "l23", 0);
+        createLine(network, b3, b1, "l31", 0);
+
+        LoadFlowResult result = loadFlowRunner.run(network, parameters);
+
+        assertTrue(result.isOk());
+        assertVoltageEquals(1, b1);
+        assertVoltageEquals(1, b2);
+        assertVoltageEquals(1, b3);
+        assertAngleEquals(0, b1);
+        assertAngleEquals(0, b2);
+        assertAngleEquals(0, b3);
+    }
 }