Introduce voltage sensitivity #253

Djazouli · 2021-03-23T17:14:39Z

Signed-off-by: Gael Macherel [email protected]

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

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

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

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: Gael Macherel <[email protected]>

Djazouli · 2021-03-23T17:15:17Z

We will need to update the core version once the sensitivity is released on core's side

Djazouli · 2021-03-23T17:16:51Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

@@ -55,6 +54,27 @@ protected AbstractSensitivityAnalysis(MatrixFactory matrixFactory) {
        this.matrixFactory = Objects.requireNonNull(matrixFactory);
    }

+    protected static Terminal getEquipmentRegulatingTerminal(Network network, String equipmentId) {


@annetill Here I am trying to get the terminal that corresponds to the equipmentId, but I am not sure of the way to do it

I have pushed a commit to help you (but the code could be improved if you want). Indeed it was what I have tried to avoid with another design of the factor, without any success from my colleagues :-)

Djazouli · 2021-03-23T17:21:13Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

+                functionLfBranch = null;
+                Bus bus = ((BusVoltagePerTargetV) factor).getFunction().getBusRef().resolve(network).orElseThrow();
+                functionLfBus = lfNetwork.getBusById(bus.getId());
+                equationTerm = equationSystem.getEquationTerm(ElementType.BUS, functionLfBus.getNum(), EquationTerm.VariableEquationTerm.class);


@geofjamg @annetill I would like to do the same thing as the branches (like using functionLfBus.getV() to get an Evaluable), but I am not sure if the Evaluable should be the bus_V equation, or the bus_V equation term ? Moreover, currently, getV() returns v / nominalV, so I am not sure whether the nominal V should be included in the Evaluable or not

You can use the BUS_V equation term.
No the / nominalV should not be included and just be careful to add it when updating the IIDM network (LfBusImpl.updateState method)

Signed-off-by: Anne Tilloy <[email protected]>

geofjamg · 2021-03-26T12:26:08Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

+                functionLfBranch = null;
+                Bus bus = ((BusVoltagePerTargetV) factor).getFunction().getBusRef().resolve(network).orElseThrow();
+                functionLfBus = lfNetwork.getBusById(bus.getId());
+                equationTerm = equationSystem.getEquationTerm(ElementType.BUS, functionLfBus.getNum(), EquationTerm.VariableEquationTerm.class);


You can use the BUS_V equation term.
No the / nominalV should not be included and just be careful to add it when updating the IIDM network (LfBusImpl.updateState method)

geofjamg · 2021-03-26T12:28:15Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

@@ -111,7 +146,9 @@ protected JacobianMatrix createJacobianMatrix(EquationSystem equationSystem, Vol
        // Wrap factors in specific class to have instant access to their branch, and their equation term
        private final SensitivityFactor factor;

-        private final LfBranch functionLfBranch;
+        private final LfBranch functionLfBranch; // todo: create two classes: branch function and bus function ?


Ideally yes, if possible we should have another class with commons abstract for branch or bus monitored factor.

geofjamg · 2021-03-26T12:28:53Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

+
+        LfBusVoltagePerTargetV(SensitivityFactor factor, Network network, LfNetwork lfNetwork, EquationSystem equationSystem) {
+            super(factor, network, lfNetwork, equationSystem);
+            String targetVTerminalId = ((TargetV) factor.getVariable()).getEquipmentId();


This is not a terminal id, but an element id

geofjamg · 2021-03-26T12:30:59Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

+        LfBusVoltagePerTargetV(SensitivityFactor factor, Network network, LfNetwork lfNetwork, EquationSystem equationSystem) {
+            super(factor, network, lfNetwork, equationSystem);
+            String targetVTerminalId = ((TargetV) factor.getVariable()).getEquipmentId();
+            Terminal regulatingTerminal = getEquipmentRegulatingTerminal(network, targetVTerminalId);


The factor/bus ref is crappy, this why you are doing all this ugly code. Here factor should provide you a TerminaRef directly.

Signed-off-by: Gael Macherel <[email protected]>

geofjamg · 2021-03-30T12:36:55Z

@Djazouli Now that we have a sensi internal API, I propose that for this PR, you don't care about powsybl sensi API and just make voltage sensi available from internal API. In Gridpy I made an evolution to directly connect to sensi internal API, so even without new powsybl API addition I will be able to use it.

geofjamg · 2021-03-30T13:04:34Z

@Djazouli Now that we have a sensi internal API, I propose that for this PR, you don't care about powsybl sensi API and just make voltage sensi available from internal API. In Gridpy I made an evolution to directly connect to sensi internal API, so even without new powsybl API addition I will be able to use it.

The idea would be to merge this, before switching to next powsybl release. Adaptation to new factor from powsybl api would be done in another PR.

Djazouli · 2021-03-30T13:35:32Z

@Djazouli Now that we have a sensi internal API, I propose that for this PR, you don't care about powsybl sensi API and just make voltage sensi available from internal API. In Gridpy I made an evolution to directly connect to sensi internal API, so even without new powsybl API addition I will be able to use it.

Sweet idea, I'll do it

Signed-off-by: Gael Macherel <[email protected]>

geofjamg · 2021-03-30T21:22:58Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

        private final String variableId;

-        protected final LfBranch functionLfBranch;
+        ILfSensitivityFunction function;


LfBranch and LfBus has a common base class LfElement. Would it be possible to use it here in order to avoid adding ILfSensitivityFunction?

Maybe we could try in this PR to adapt LfFactor to internal API. To avoid LfFactor subclassing hell (with all function/variable combinations), we could try to only have one LfFactor class and one LfFactorGroup. So having LfElement type for variable and function.

So, we would to store a LfElement + a SensitivityFunctionType instead of having a class wrapping the element for each Function type ? And then, to get the equation term from that, we write a big switch statement, instead of overriding methods ? (and do the same thing for the variables)

We can try to see what it would look like.

Signed-off-by: Gael Macherel <[email protected]>

geofjamg · 2021-04-01T06:49:55Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

    protected JacobianMatrix createJacobianMatrix(EquationSystem equationSystem, VoltageInitializer voltageInitializer) {
        double[] x = equationSystem.createStateVector(voltageInitializer);
        equationSystem.updateEquations(x);
        return new JacobianMatrix(equationSystem, matrixFactory);
    }

-    static class LfSensitivityFactor {
+    abstract static class AbstractLfSensitivityFactor {


Thanks, for this great refactoring! Maybe we can keep a LfSensitivityFactor interface in addition to the AbstractLfSensitivityFactor to avoid using AbstractLfSensitivityFactor everywhere in the code (Abstract class are not supposed to be used as an API)?

Signed-off-by: Gael Macherel <[email protected]>

annetill · 2021-04-01T08:08:31Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

        public EquationTerm getEquationTerm() {
-            throw new NotImplementedException("getEquationTerm should have an override");
+            switch (functionType) {
+                case BRANCH_ACTIVE_POWER:


Just a small remark here: maybe it could be better to use the terminology we already introduced: BRANCH_P, BRANCH_I (I have it on a working branch for transformer in current limiter mode) and BUS_V?

I would keep it like this as it is supposed one day to move to core as API V2 and as an official API is it more meaningful to keep BRANCH_ACTIVE_POWER

annetill · 2021-04-01T13:50:25Z

src/main/java/com/powsybl/openloadflow/sensi/AbstractSensitivityAnalysis.java

+        if (staticVarCompensator != null) {
+            return staticVarCompensator.getRegulatingTerminal();
+        }
+        TwoWindingsTransformer t2wt = network.getTwoWindingsTransformer(equipmentId);


This PR is indeed not working with transformers as we don't have any PV bus linked to the transformer regulation. In your test, the PV bus is linked to the generator. For phase tap changers, we have activated the phase variable. Here, we have to active the rho variable and the linked bus_v equation I think.

Signed-off-by: Gael Macherel <[email protected]>

annetill · 2021-04-02T15:14:16Z

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

@@ -263,7 +263,7 @@ private static void createBranchActivePowerTargetEquation(LfBranch branch, Discr

    private static void createDiscreteVoltageControlEquation(LfBus bus,  VariableSet variableSet, EquationSystem equationSystem) {
        if (bus.isDiscreteVoltageControlled()) {
-            EquationTerm vTerm = EquationTerm.createVariableTerm(bus, VariableType.BUS_V, variableSet, bus.getV().eval());
+            EquationTerm vTerm = EquationTerm.createVariableTerm(bus, VariableType.BUS_V, variableSet, bus.getDiscreteVoltageControl().getTargetValue());


@geofjamg I think it is bus.getV().eval()

Using a numeric value of a state variable in the equation system is not correct.
bus.getDiscreteVoltageControl().getTargetValue() is better, but not correct too. It should be redesign to move it in the RHS.

After thinking again, I am not sure to understand why do we need to set an initial value for equation term at this place. Normally equation initial value should be computed from initial state vector. To investigate before merging this PR.

Yes indeed I understand now the purpose of this new term and we have to investigate why we need it now.

The initial value is only used if the voltage initializer type is set to PREVIOUS_VALUES. (because it causes a call to lfBus.getV().eval() ). Otherwise it is indeed replaced immediately by the value from the state vector.

But with PREVIOUS_VALUES the bus voltage is initialized from IIDM values (if defined). Why do you need to override this value?

The PREVIOUS_VALUES was initialized with LfBus.getV() (now lfBus.getV().eval()). The issue is that after the creation of the system, lfBus.getV() returns the EquationTerm representing V in the system, and not the value found in the IIDM, so we need to pass the value from the IIDM to the EquationTerm so it does not get lost. (or we could also try to call the voltage initialization before the creation of the equation system)

Ok I understand, so @annetill is right in her comment, it should be bus.getV().eval() in that case to really init from IIDM network state as it was before.

Signed-off-by: Gael Macherel <[email protected]>

annetill · 2021-04-06T11:16:16Z

src/main/java/com/powsybl/openloadflow/equations/EquationTerm.java

        @Override
        public void write(Writer writer) throws IOException {
            variables.get(0).write(writer);
        }
    }

    static VariableEquationTerm createVariableTerm(LfElement element, VariableType variableType, VariableSet variableSet) {
+        return createVariableTerm(element, variableType, variableSet, 0d);


I think it will be better to put Double.NaN here.

Signed-off-by: Gael Macherel <[email protected]>

…ite the equation term Signed-off-by: Gael Macherel <[email protected]>

Signed-off-by: Gael Macherel <[email protected]>

sonarqubecloud · 2021-04-07T17:26:44Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
14 Code Smells

90.3% Coverage
0.0% Duplication

introduce voltage sensitivity

a15b447

Signed-off-by: Gael Macherel <[email protected]>

Djazouli requested a review from annetill March 23, 2021 17:14

Djazouli commented Mar 23, 2021

View reviewed changes

Djazouli and others added 3 commits March 23, 2021 18:28

Merge branch 'master' into V_sensitivity

fbf7d96

Merge branch 'master' into V_sensitivity

3a8723f

Fix regulating terminal search.

6019405

Signed-off-by: Anne Tilloy <[email protected]>

geofjamg requested changes Mar 26, 2021

View reviewed changes

Djazouli added 2 commits March 29, 2021 16:18

rename terminal to equipment

1ea2b3e

Signed-off-by: Gael Macherel <[email protected]>

Use Evaluable for bus V

27e3af5

Signed-off-by: Gael Macherel <[email protected]>

Djazouli force-pushed the V_sensitivity branch from 5ed770a to 27e3af5 Compare March 30, 2021 08:17

Djazouli added 3 commits March 30, 2021 11:12

Create a wrapper around SensitivityFactors' functions

be69e37

Signed-off-by: Gael Macherel <[email protected]>

Merge branch 'master' into V_sensitivity

ecef5ec

Signed-off-by: Gael Macherel <[email protected]>

merge fix

68098f2

Signed-off-by: Gael Macherel <[email protected]>

Djazouli and others added 10 commits March 30, 2021 17:14

test fix

9e626f6

Signed-off-by: Gael Macherel <[email protected]>

Remove dependency to core 4.1.0

a66db4d

Signed-off-by: Gael Macherel <[email protected]>

remove pom dependency to 4.1.0

1a44235

Signed-off-by: Gael Macherel <[email protected]>

fix bug and some code smells

fb9fabe

Signed-off-by: Gael Macherel <[email protected]>

Merge branch 'master' into V_sensitivity

3b53d34

add twt test

4f9d343

Signed-off-by: Gael Macherel <[email protected]>

fix verify

3d85163

Signed-off-by: Gael Macherel <[email protected]>

add vsc test

6810c1a

Signed-off-by: Gael Macherel <[email protected]>

throw exception on three windings transformers

d855c83

Signed-off-by: Gael Macherel <[email protected]>

throw exception on 3wt

ef9f75f

Signed-off-by: Gael Macherel <[email protected]>

geofjamg reviewed Mar 30, 2021

View reviewed changes

remove redundancy

92a165e

Signed-off-by: Gael Macherel <[email protected]>

Djazouli added 2 commits March 31, 2021 22:51

Refactor LfSensitivityFactors

f275c20

Signed-off-by: Gael Macherel <[email protected]>

fix code smells

f2b605c

Signed-off-by: Gael Macherel <[email protected]>

geofjamg requested changes Apr 1, 2021

View reviewed changes

extract interfaces from abstract classes

f9db7fe

Signed-off-by: Gael Macherel <[email protected]>

Djazouli changed the title ~~[WIP] introduce voltage sensitivity~~ Introduce voltage sensitivity Apr 1, 2021

annetill reviewed Apr 1, 2021

View reviewed changes

Djazouli added 2 commits April 2, 2021 14:49

throw error on transformer for V sensitivity

ba99a38

Signed-off-by: Gael Macherel <[email protected]>

remove so-called bug

52a11a4

Signed-off-by: Gael Macherel <[email protected]>

geofjamg approved these changes Apr 2, 2021

View reviewed changes

annetill reviewed Apr 2, 2021

View reviewed changes

use bus.getV() to initialize discrete controlled buses' equations

37c658a

Signed-off-by: Gael Macherel <[email protected]>

annetill approved these changes Apr 6, 2021

View reviewed changes

annetill reviewed Apr 6, 2021

View reviewed changes

Djazouli added 3 commits April 6, 2021 15:37

init value to NaN if not specified

7c080d4

Signed-off-by: Gael Macherel <[email protected]>

Do not set V to NaN after a DC initialization because it would overwr…

5c0e185

…ite the equation term Signed-off-by: Gael Macherel <[email protected]>

init v term on non impedant branch

c447507

Signed-off-by: Gael Macherel <[email protected]>

annetill approved these changes Apr 8, 2021

View reviewed changes

annetill merged commit c2b0a30 into master Apr 8, 2021

annetill deleted the V_sensitivity branch April 8, 2021 09:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce voltage sensitivity #253

Introduce voltage sensitivity #253

Djazouli commented Mar 23, 2021

Djazouli commented Mar 23, 2021

Djazouli Mar 23, 2021

annetill Mar 24, 2021

Djazouli Mar 23, 2021

geofjamg Mar 26, 2021

geofjamg Mar 26, 2021

geofjamg Mar 26, 2021

geofjamg Mar 26, 2021

geofjamg Mar 26, 2021

geofjamg commented Mar 30, 2021

geofjamg commented Mar 30, 2021

Djazouli commented Mar 30, 2021

geofjamg Mar 30, 2021

geofjamg Mar 30, 2021

Djazouli Mar 31, 2021

geofjamg Mar 31, 2021

geofjamg Apr 1, 2021

annetill Apr 1, 2021

geofjamg Apr 1, 2021 •

edited

Loading

annetill Apr 1, 2021

annetill Apr 2, 2021

geofjamg Apr 2, 2021

geofjamg Apr 5, 2021

annetill Apr 6, 2021

Djazouli Apr 6, 2021

geofjamg Apr 6, 2021

Djazouli Apr 6, 2021

geofjamg Apr 6, 2021

annetill Apr 6, 2021

sonarqubecloud bot commented Apr 7, 2021

Introduce voltage sensitivity #253

Introduce voltage sensitivity #253

Conversation

Djazouli commented Mar 23, 2021

Djazouli commented Mar 23, 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

geofjamg commented Mar 30, 2021

geofjamg commented Mar 30, 2021

Djazouli commented Mar 30, 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

geofjamg Apr 1, 2021 • edited Loading

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

sonarqubecloud bot commented Apr 7, 2021

geofjamg Apr 1, 2021 •

edited

Loading