Fix reference flow that were not computed and set as NaN when variable element is not in the main component but function element is #349

Hadrien-Godard · 2021-08-30T13:05:59Z

Signed-off-by: Hadrien hadrien.godard@artelys.com

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)

…e element factor does not exist but function element does. Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill

Can you please check my two last commits and increase the test coverage ?

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

…y loss. There is no need to use predefined results for reference values of SKIP_ONLY_VARIABLE factors Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

annetill · 2021-09-10T11:43:23Z

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

@@ -50,6 +50,8 @@ private void calculateSensitivityValues(List<LfSensitivityFactor<AcVariableType,
                                            String contingencyId, int contingencyIndex, SensitivityValueWriter valueWriter) {
        Set<LfSensitivityFactor<AcVariableType, AcEquationType>> lfFactorsSet = new HashSet<>(lfFactors);
        lfFactors.stream().filter(factor -> factor.getStatus() == LfSensitivityFactor.Status.ZERO).forEach(factor -> valueWriter.write(factor.getContext(), contingencyId, contingencyIndex, 0, Double.NaN));
+        lfFactors.stream().filter(factor -> factor.getStatus() == LfSensitivityFactor.Status.SKIP_ONLY_VARIABLE).forEach(factor -> valueWriter.write(factor.getContext(), contingencyId, contingencyIndex, 0, unscaleFunction(factor, factor.getFunctionReference())));


0 for value is not consistent with the Double.NaN chosen as value for SKIP factor.

annetill · 2021-09-10T11:45:08Z

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

+        }
+
+        valueWriter.write(factor.getContext(), contingency != null ? contingency.getContingency().getId() : null, contingency != null ? contingency.getIndex() : -1,
+                0d, unscaleFunction(factor, flowValue));


Here we choose 0 for sensitivity value, it is what you want? Or is it better to choose Double.NaN?

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

…ltRef to handle the special case where we know that the sensitivity is null (variable and function elements are in different connected componants in the pre contingency network), but the reference flow can be computed in the pre contingency network, and not in the post contingency one. Here the sensitivity equals 0 but the reference flow should be set to NaN for the post contingency case. Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Replace SKIP_ONLY_VARIABLE by VALID_REFERENCE Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

…nctionelementexists' into fix_referenceflowscomputedwhenfunctionelementexists

…entexists

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill · 2021-09-14T08:49:49Z

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

-                connectivityAnalysisResult.setPredefinedResults();
+                lfFactorsForContingencies.forEach(factor -> factor.setSensitivityValuePredefinedResult(null));
+                lfFactorsForContingencies.forEach(factor -> factor.setFunctionPredefinedResult(null));
+                connectivityAnalysisResult.setSensitivityValuePredefinedResults();


Why do you need these two lines?

annetill · 2021-09-14T08:54:46Z

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

+                    factor.setSensitivityValuePredefinedResult(0d);
+                }
+                if (!variableConnected && !functionConnected) {
+                    // SKIP status


Here I think that this level of details is not needed: in case of factor with SKIP status, in pre-contingency state, you write NaN for sensi and NaN for function. But you introduce a new level of detail after a contingency. It is not coherent and I think that if it is more right, it is less perfomant.

I agree with you.
Indeed, in precontingency network, skip status is used when both variable and function are disconnected from slack and the sensitivity is set to NaN.
But in case of a contingency breaking connectivity we treat the case where both function and element are disconnected from slack and are in different connected components with a 0 value for the sensitivity.

This behavior is not coherent.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill · 2021-09-14T09:17:15Z

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

@@ -129,7 +134,9 @@ protected static Terminal getEquipmentRegulatingTerminal(Network network, String

        boolean areVariableAndFunctionDisconnected(GraphDecrementalConnectivity<LfBus> connectivity);

-        boolean isConnectedToComponent(Set<LfBus> connectedComponent);
+        boolean isVariableConnectedToSlackComponent(Set<LfBus> connectedComponent);


What is for me a bit hard to understand if that these two functions are not used for pre-contingency state. It is an incoherent behavior, no?

Those two functions are not used for pre-contingency state indeed and we only check if variable and function element are in slack connected component. This may result to an incoherent behavior regarding SKIP status as discussed in another comment.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill · 2021-09-14T09:21:16Z

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

-        skippedFactors.forEach(factor -> valueWriter.write(factor.getContext(), null, -1, 0, Double.NaN));
+        // SKIP factors are for factors where both variable and function elements are not in the main connected componant.
+        // Therefore, their sensitivity and reference values are set to NaN.
+        skippedFactors.forEach(factor -> valueWriter.write(factor.getContext(), null, -1, Double.NaN, Double.NaN));


See, for base case, SKIP is always NaN and NaN.

…ation. Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

…ncy instead of NaN Signed-off-by: Hadrien <hadrien.godard@artelys.com>

…entexists

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

…entexists

sonarqubecloud · 2021-09-23T14:32:49Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

88.3% Coverage
0.0% Duplication

Fix reference flow that were not computed and set as NaN when variabl…

efca250

…e element factor does not exist but function element does. Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Hadrien-Godard requested a review from annetill August 30, 2021 13:05

annetill added 2 commits August 30, 2021 19:48

Small refactoring.

05345dc

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Refactoring.

79664bf

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill requested changes Aug 31, 2021

View reviewed changes

Hadrien-Godard added 5 commits September 1, 2021 11:11

Increase test cover

536b6d2

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Increase test coverage

d092288

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Increase test cover

55e805f

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Increase test coverage and set SKIP sensitivities to NaN

2d5b037

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

SKIP_ONLY_VARIABLE factors were not computed in case of a connectivit…

5f356b0

…y loss. There is no need to use predefined results for reference values of SKIP_ONLY_VARIABLE factors Signed-off-by: Hadrien <hadrien.godard@artelys.com>

annetill added the PR: next-release label Sep 9, 2021

annetill and others added 3 commits September 9, 2021 16:25

Merge.

fce5c17

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Fix code smells.

1c7f320

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Improve test coverage and removed useless lines in AcSensitivityAnalysis

4470b46

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

annetill reviewed Sep 10, 2021

View reviewed changes

Hadrien-Godard and others added 14 commits September 10, 2021 13:51

Change how test are performed in filters

611cd69

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Fix code smells.

f41fd2b

Replace SKIP_ONLY_VARIABLE by VALID_REFERENCE Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Adding some comments

0b66ebc

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Change names.

1a07237

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Try to refactor.

5737cab

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Fix.

618bfc7

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Rename functions.

c1b5f48

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Correction of createBranchSensitivityValue and some TUs

18115f1

Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Merge remote-tracking branch 'origin/fix_referenceflowscomputedwhenfu…

67caffa

…nctionelementexists' into fix_referenceflowscomputedwhenfunctionelementexists

Merge branch 'master' into fix_referenceflowscomputedwhenfunctionelem…

6d1470b

…entexists

Slight changes.

5f5bd68

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Fix code smells.

78ad9c3

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Remove useless code.

9236949

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill reviewed Sep 14, 2021

View reviewed changes

Simplify predefined results for post contingency state.

694a0e6

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill reviewed Sep 14, 2021

View reviewed changes

Remove useless function.

161d9a2

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

annetill reviewed Sep 14, 2021

View reviewed changes

annetill and others added 3 commits September 14, 2021 16:42

Fix when the monitored branch is part of the contingency in AC calcul…

76dab0e

…ation. Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Small change to set function value to zero for an element in continge…

2aa96b4

…ncy instead of NaN Signed-off-by: Hadrien <hadrien.godard@artelys.com>

Merge branch 'master' into fix_referenceflowscomputedwhenfunctionelem…

5298607

…entexists

phiedw approved these changes Sep 17, 2021

View reviewed changes

annetill removed the PR: next-release label Sep 20, 2021

annetill and others added 3 commits September 20, 2021 10:07

Merge branch 'master' into fix_referenceflowscomputedwhenfunctionelem…

135e4a7

…entexists

Clean code.

92d740f

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>

Merge branch 'master' into fix_referenceflowscomputedwhenfunctionelem…

6defa5b

…entexists

flo-dup merged commit ac2ef5f into master Sep 23, 2021

flo-dup deleted the fix_referenceflowscomputedwhenfunctionelementexists branch September 23, 2021 14:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix reference flow that were not computed and set as NaN when variable element is not in the main component but function element is #349

Fix reference flow that were not computed and set as NaN when variable element is not in the main component but function element is #349

Hadrien-Godard commented Aug 30, 2021

annetill left a comment

annetill Sep 10, 2021 •

edited

Loading

annetill Sep 10, 2021

annetill Sep 14, 2021

annetill Sep 14, 2021

Hadrien-Godard Sep 14, 2021

annetill Sep 14, 2021

Hadrien-Godard Sep 14, 2021

annetill Sep 14, 2021

sonarqubecloud bot commented Sep 23, 2021

Fix reference flow that were not computed and set as NaN when variable element is not in the main component but function element is #349

Fix reference flow that were not computed and set as NaN when variable element is not in the main component but function element is #349

Conversation

Hadrien-Godard commented Aug 30, 2021

annetill left a comment

Choose a reason for hiding this comment

annetill Sep 10, 2021 • edited Loading

Choose a reason for hiding this comment

annetill Sep 10, 2021

Choose a reason for hiding this comment

annetill Sep 14, 2021

Choose a reason for hiding this comment

annetill Sep 14, 2021

Choose a reason for hiding this comment

Hadrien-Godard Sep 14, 2021

Choose a reason for hiding this comment

annetill Sep 14, 2021

Choose a reason for hiding this comment

Hadrien-Godard Sep 14, 2021

Choose a reason for hiding this comment

annetill Sep 14, 2021

Choose a reason for hiding this comment

sonarqubecloud bot commented Sep 23, 2021

annetill Sep 10, 2021 •

edited

Loading