Network listener #215

geofjamg · 2021-02-06T20:19:45Z

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
No

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature

What is the new behavior (if this is a feature change)?
In order to centralize, network modification to equation system change, a listener has been added to the LF network and is used by AC equation system to auto update

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)

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

# Conflicts: # src/main/java/com/powsybl/openloadflow/network/LfNetwork.java # src/main/java/com/powsybl/openloadflow/sa/OpenSecurityAnalysis.java

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

annetill · 2021-02-10T10:59:04Z

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

+
+                // also create an inactive equation to have a1 variable constant
+                LfBranch controller = phaseControl.getController();
+                equationSystem.createEquation(controller.getNum(), EquationType.BRANCH_ALPHA1)


The equation type BRANCH_ALPHA1 you introduce recently is in fact very confusing for me as we already a variable type BRANCH_ALPHA1. And here I am not sure to get what you have in mind to add these lines.

It is to keep the same matrix size?

I reverted the phase control refactoring. I will add it in another PR and will explain better why.

annetill · 2021-02-10T11:00:00Z

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

+
+            @Override
+            public void onPhaseControlModeChange(DiscretePhaseControl phaseControl, DiscretePhaseControl.Mode oldMode, DiscretePhaseControl.Mode newMode) {
+                boolean on = newMode != DiscretePhaseControl.Mode.OFF;


Here, the code on master is:

LfBranch controllerBranch = branch.getDiscretePhaseControl().getController(); Variable a1 = context.getVariableSet().getVariable(controllerBranch.getNum(), VariableType.BRANCH_ALPHA1); a1.setActive(false); Equation t = context.getEquationSystem().createEquation(branch.getNum(), EquationType.BRANCH_P); t.setActive(false);

Why we don't have any variable management?

I reverted the phase control refactoring. I will add it in another PR and will explain better why.

annetill · 2021-02-10T11:02:30Z

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

+
+                    for (LfBranch controllerBranch : bus.getDiscreteVoltageControl().getControllers()) {
+                        // de-activate r1 variable
+                        Variable r1 = variableSet.getVariable(controllerBranch.getNum(), VariableType.BRANCH_RHO1);


Here you manage the variable BRANCH_RHO1, so why BRANCH_ALPHA1 has disappeared?

I reverted the phase control refactoring. I will add it in another PR and will explain better why.

annetill · 2021-02-10T11:04:39Z

src/main/java/com/powsybl/openloadflow/sa/OpenSecurityAnalysis.java

@@ -450,4 +451,21 @@ private void reactivateEquations(List<Equation> deactivatedEquations, List<Equat
        return connectivity;
    }

+    /**


Why do you have these "new" lines?

they are old lines in fact which have been moved recently, so they can be removed

No more new lines

flo-dup · 2021-02-10T13:32:35Z

src/main/java/com/powsybl/openloadflow/sa/OpenSecurityAnalysis.java

@@ -450,4 +451,21 @@ private void reactivateEquations(List<Equation> deactivatedEquations, List<Equat
        return connectivity;
    }

+    /**


they are old lines in fact which have been moved recently, so they can be removed

flo-dup · 2021-02-10T13:51:25Z

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

@@ -350,6 +373,70 @@ public static EquationSystem create(LfNetwork network, VariableSet variableSet,
        createBusEquations(network, variableSet, creationParameters, equationSystem);
        createBranchEquations(network, variableSet, creationParameters, equationSystem);

+        network.addListener(new LfNetworkListener() {
+            @Override
+            public void onVoltageControlChange(LfBus bus, boolean oldVoltageControl, boolean newVoltageControl) {


oldVoltageControl not very necessary as we always have oldVoltageControl == !newVoltageControl, but conform to other methods so...

Agree this is ugly, I removed oldVoltageControl

flo-dup · 2021-02-10T13:58:34Z

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

@@ -350,6 +373,70 @@ public static EquationSystem create(LfNetwork network, VariableSet variableSet,
        createBusEquations(network, variableSet, creationParameters, equationSystem);
        createBranchEquations(network, variableSet, creationParameters, equationSystem);

+        network.addListener(new LfNetworkListener() {


I'm not very fond of anonymous classes, don't you think this one is big enough to be explicit?

Ok, I moved everything in a AcEquationSystemUpdater class.

flo-dup · 2021-02-10T15:58:45Z

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

+ */
+public abstract class AbstractElement {
+
+    protected final LfNetwork network;


Should we give the access to the whole LfNetwork to all elements or wouldn't a Supplier<LfNetworkListener> be enough?

I think it could be useful to go back to network from an element

flo-dup · 2021-02-10T16:02:50Z

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

@@ -18,17 +20,16 @@
 /**
 * @author Geoffroy Jamgotchian <geoffroy.jamgotchian at rte-france.com>
 */
-public class LfShuntImpl implements LfShunt {
+public class LfShuntImpl extends AbstractElement implements LfShunt {


For now there's no use for LfShuntImpl to extend AbstractElement, but I suppose you're anticipating some feature?
I'm asking as if we don't extend it, it would give more consistency to AbstractElement in a way (and some more LfBus / LfBranch common methods could be added)

Honestly, I only added AbstractElement to fix a Sonar duplicated code issue at the moment. But for sure we can probably share more code. I Propose to do it in an other PR as it was really not the aim of this one.

# Conflicts: # src/main/java/com/powsybl/openloadflow/ac/outerloop/AcloadFlowEngine.java # src/main/java/com/powsybl/openloadflow/ac/outerloop/OuterLoopContext.java # src/main/java/com/powsybl/openloadflow/sa/OpenSecurityAnalysis.java # src/main/java/com/powsybl/openloadflow/util/BusState.java

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

geofjamg · 2021-02-28T14:24:25Z

@annetill @floriand-e2r I removed the phase control refactoring from this PR to only keep the listener part. So this PR now """"only""""" contains code move and is supposed to be functionnally equivalent. I will create to dedicated PR to refactor the AC equation system to avoid variable de-activation.

sonarqubecloud · 2021-02-28T14:29:18Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

94.2% Coverage
0.0% Duplication

annetill

It is really pretty like this and for sure more easy to use as the AC equation system is hidden. We have to make an issue not to forget to implement it for DC equation system when we will be able to have a regulation on phase tap changers.

geofjamg added 5 commits February 6, 2021 20:34

Add network listener and refactor outer loops

72094f7

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>

Remove equation system access from outer loops

8e2f59f

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>

Cleanup

5b0a534

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>

Cleanup

7aa4e96

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>

Add checks

77f7f9d

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>

geofjamg requested review from annetill and flo-dup February 7, 2021 20:58

geofjamg added 5 commits February 7, 2021 22:36

Add AbstractElement

e95f256

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

Fix Sonar issue

7b42ddb

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

Refactor phase control equations

f359139

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

Merge branch 'master' into network_listener

c894a73

# Conflicts: # src/main/java/com/powsybl/openloadflow/network/LfNetwork.java # src/main/java/com/powsybl/openloadflow/sa/OpenSecurityAnalysis.java

Merge

791e0de

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

annetill reviewed Feb 10, 2021

View reviewed changes

flo-dup reviewed Feb 10, 2021

View reviewed changes

geofjamg added 5 commits February 28, 2021 15:03

Merge

137e285

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

Revert phase control refactoring

a549324

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

Remove oldVoltageControl

d14ab86

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

Add AcEquationSystemUpdater

eaff81e

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>

geofjamg requested review from annetill and flo-dup February 28, 2021 14:26

annetill approved these changes Mar 1, 2021

View reviewed changes

geofjamg merged commit 48b763a into master Mar 1, 2021

geofjamg deleted the network_listener branch March 1, 2021 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Network listener #215

Network listener #215

geofjamg commented Feb 6, 2021

annetill Feb 10, 2021

annetill Feb 10, 2021

geofjamg Feb 28, 2021

annetill Feb 10, 2021

geofjamg Feb 28, 2021

annetill Feb 10, 2021

geofjamg Feb 28, 2021

annetill Feb 10, 2021

flo-dup Feb 10, 2021

geofjamg Feb 28, 2021

flo-dup Feb 10, 2021

flo-dup Feb 10, 2021

geofjamg Feb 28, 2021

flo-dup Feb 10, 2021

geofjamg Feb 28, 2021

flo-dup Feb 10, 2021

geofjamg Feb 28, 2021

flo-dup Feb 10, 2021

geofjamg Feb 28, 2021

geofjamg commented Feb 28, 2021

sonarqubecloud bot commented Feb 28, 2021

annetill left a comment

Network listener #215

Network listener #215

Conversation

geofjamg commented Feb 6, 2021

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

Choose a reason for hiding this comment

geofjamg commented Feb 28, 2021

sonarqubecloud bot commented Feb 28, 2021

annetill left a comment

Choose a reason for hiding this comment