Improve performance of incremental outer loops #895

caioluke · 2023-11-08T17:09:36Z

Please check if the PR fulfills these requirements

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?

What kind of change does this PR introduce?

Performance enhancement

What is the current behavior?

We currently calculate sensitivity for all controllers of the network, even though we won't need them all.

What is the new behavior (if this is a feature change)?
Sensitivities are only calculated for the necessary controllers (outside target/deadband).

Does this PR introduce a breaking change or deprecate an API?

No

Other information:

Signed-off-by: Caio Luke <caioluke97@gmail.com>

...main/java/com/powsybl/openloadflow/ac/outerloop/IncrementalShuntVoltageControlOuterLoop.java

...ava/com/powsybl/openloadflow/ac/outerloop/IncrementalTransformerVoltageControlOuterLoop.java

Signed-off-by: Caio Luke <caioluke97@gmail.com>

geofjamg · 2023-11-09T09:41:00Z

@caioluke is this optimization based on some performance profiles? Do you have observed in a real case a significant performance improvement?

jeandemanged · 2023-11-09T11:26:19Z

@geofjamg yes we observed the issue on large networks.
Didn't check code yet but just tested it today.
Performance improvement is quite significative.

Flamegraph before (outerloop is taking more time than the NR ...):

Flamegraph after:

solution "path" unchanged but much faster (left is before fix, right after fix):

geofjamg · 2023-11-09T11:46:25Z

Indeed.
Do you have the "solve2" individual execution time (before) and how much time it is called?
Also are you able with your profiler to get JNI native function profiling (IntelliJ profiler can do it by changing the option 'Collect native calls") ?

geofjamg · 2023-11-12T14:26:36Z

I summarize the discussion I had with @caioluke:

it is indeed a good idea to pre-filter controllers that are outside of the dead band
- it really accelerates the performance, because the more the incremental outerloops are running less there is controller outside of dead band. So firsts outer loops are slow and last are very fast.
- Even if faster, we still see the sensi calculation of incremental outer loops in profiling so there is room to more optimization.
The REAL PROBLEM which is the one to solve is that we are solving a very very sparse second member. Second member is made with a matrix that has only one non zero value per column (and the number of row is the size of the system). KLU library only supports dense second member (this is the normal use so it makes sense). So in our very special way to use KLU for sensi analysis, the KLU_solve and KLU_tsolve functions are very inefficients. Thanks @caioluke @jeandemanged for pointing this I was not aware of it.
A solution would be to re-implement our own KLU_solve and KLU_tsolve function. Looking at https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/dev/KLU/Source/klu_sort.c for instance, the code is quite simple and short and could maybe be copy/pasted to work on a sparse second member (instead of a B[], a pair B_values[] and B_Index[]). Performance would be much faster, only processing non zero values.

@jeandemanged @Hadrien-Godard what do you think?

# Conflicts: # src/main/java/com/powsybl/openloadflow/ac/outerloop/IncrementalShuntVoltageControlOuterLoop.java # src/main/java/com/powsybl/openloadflow/ac/outerloop/IncrementalTransformerVoltageControlOuterLoop.java

Signed-off-by: Caio Luke <caioluke97@gmail.com>

geofjamg · 2023-11-23T11:43:46Z

...main/java/com/powsybl/openloadflow/ac/outerloop/IncrementalShuntVoltageControlOuterLoop.java

+            double diffV = controlledBus.getHighestPriorityMainVoltageControl().orElseThrow().getTargetValue() - voltageControl.getControlledBus().getV();
+            double halfTargetDeadband = getHalfTargetDeadband(voltageControl);
+            if (Math.abs(diffV) > halfTargetDeadband) {
+                List<LfShunt> controllers = voltageControl.getMergedControllerElements().stream()


We have a difference here in the way we get controllers from a controlled. In previous code, we use IncrementalContextData.getControllerElements which has a filter on voltage control type. Is it equivalent?

Yes, it is equivalent. I directly call the method for the specific type of voltage control: getShuntVoltageControl or getTransformerVoltageControl. And I also filter out the disabled controllers.

...ava/com/powsybl/openloadflow/ac/outerloop/IncrementalTransformerVoltageControlOuterLoop.java

…ts for transformer outerloop Signed-off-by: Caio Luke <caioluke97@gmail.com>

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

Signed-off-by: Caio Luke <caioluke97@gmail.com>

This reverts commit 121c9f8.

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

annetill · 2023-11-24T14:04:36Z

...ava/com/powsybl/openloadflow/ac/outerloop/IncrementalTransformerVoltageControlOuterLoop.java

@@ -191,11 +201,26 @@ private boolean adjustWithSeveralControllers(List<LfBranch> controllerBranches,
    }

    private static double getDiffV(TransformerVoltageControl voltageControl) {
-        double targetV = voltageControl.getTargetValue();
+        double targetV = voltageControl.getControlledBus().getHighestPriorityMainVoltageControl().orElseThrow().getTargetValue();


@jeandemanged and @geofjamg bug fix here. We have to check the voltage difference against the voltage control of highest priority, most of the time those of the generator and not those of the transformer.

annetill · 2023-11-24T14:07:41Z

Maybe a final performance test is needed before merging and a unit test that shows the bug.

Signed-off-by: Caio Luke <caioluke97@gmail.com>

annetill · 2023-11-24T15:47:31Z

src/test/java/com/powsybl/openloadflow/ac/AcLoadFlowTransformerControlTest.java

+                .setRegulating(true)
+                .setTapPosition(0)
+                .setRegulationTerminal(line34.getTerminal2())
+                .setTargetV(33.0);


Change this targetV to 30.0 to show the benefit of the bug fix.

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

sonarqubecloud · 2023-11-24T16:01:08Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

96.3% Coverage
0.0% Duplication

jeandemanged

all good, thanks

other improvements (see here) would be in other PRs

caioluke added 2 commits November 8, 2023 17:52

improve transfo outerloop

cb683c0

Signed-off-by: Caio Luke <caioluke97@gmail.com>

improve shunt outerloop

58b6fa8

Signed-off-by: Caio Luke <caioluke97@gmail.com>

caioluke added the enhancement New feature or request label Nov 8, 2023

caioluke requested a review from jeandemanged November 8, 2023 17:09

caioluke self-assigned this Nov 8, 2023

caioluke changed the title ~~[WIP] Improve performance of outerloops with sensitivity (Transformers and Shunt)~~ Improve performance of outerloops with sensitivity (Transformers and Shunt) Nov 8, 2023

geofjamg requested changes Nov 8, 2023

View reviewed changes

...main/java/com/powsybl/openloadflow/ac/outerloop/IncrementalShuntVoltageControlOuterLoop.java Outdated Show resolved Hide resolved

...ava/com/powsybl/openloadflow/ac/outerloop/IncrementalTransformerVoltageControlOuterLoop.java Outdated Show resolved Hide resolved

remove useless code

394480e

Signed-off-by: Caio Luke <caioluke97@gmail.com>

caioluke requested a review from geofjamg November 9, 2023 09:32

Merge branch 'main' into improve_sentivity_outerloops

06c8910

caioluke and others added 5 commits November 20, 2023 12:23

Merge branch 'main' into improve_sentivity_outerloops

b24fea6

# Conflicts: # src/main/java/com/powsybl/openloadflow/ac/outerloop/IncrementalShuntVoltageControlOuterLoop.java # src/main/java/com/powsybl/openloadflow/ac/outerloop/IncrementalTransformerVoltageControlOuterLoop.java

transfo outerloop ok

d94a0f3

Signed-off-by: Caio Luke <caioluke97@gmail.com>

shunt outerloop ok

37b978a

Signed-off-by: Caio Luke <caioluke97@gmail.com>

shunt outerloop now is ok

75afffe

Signed-off-by: Caio Luke <caioluke97@gmail.com>

Merge branch 'main' into improve_sentivity_outerloops

0c8784e

geofjamg reviewed Nov 23, 2023

View reviewed changes

caioluke and others added 10 commits November 23, 2023 15:37

Merge branch 'main' into improve_sentivity_outerloops

221aaa2

Merge branch 'main' into improve_sentivity_outerloops

b7d7535

Merge branch 'main' into improve_sentivity_outerloops

ec17cb4

Merge branch 'main' into improve_sentivity_outerloops

7be85a9

Merge branch 'main' into improve_sentivity_outerloops

1995b01

proposition of refactoring getControlledBuses and getControllerElemen…

5e560d0

…ts for transformer outerloop Signed-off-by: Caio Luke <caioluke97@gmail.com>

Proposal.

7139686

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

wip

121c9f8

Signed-off-by: Caio Luke <caioluke97@gmail.com>

Revert "wip"

dfa20cd

This reverts commit 121c9f8.

Fix.

39242b2

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

Fix.

4d4fed2

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

annetill reviewed Nov 24, 2023

View reviewed changes

annetill approved these changes Nov 24, 2023

View reviewed changes

jeandemanged approved these changes Nov 24, 2023

View reviewed changes

add test for transformerIncrementalOuterloop

f2780f6

Signed-off-by: Caio Luke <caioluke97@gmail.com>

annetill reviewed Nov 24, 2023

View reviewed changes

Fix unit test.

592a1a2

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

annetill changed the title ~~Improve performance of outerloops with sensitivity (Transformers and Shunt)~~ Improve performance of incremental outer loops Nov 24, 2023

geofjamg approved these changes Nov 24, 2023

View reviewed changes

jeandemanged approved these changes Nov 24, 2023

View reviewed changes

jeandemanged merged commit 3321a02 into main Nov 24, 2023
6 checks passed

jeandemanged deleted the improve_sentivity_outerloops branch November 24, 2023 17:38

jeandemanged mentioned this pull request Mar 29, 2024

Improve incremental outerloops performance #1007

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance of incremental outer loops #895

Improve performance of incremental outer loops #895

caioluke commented Nov 8, 2023 •

edited by jeandemanged

Loading

geofjamg commented Nov 9, 2023

jeandemanged commented Nov 9, 2023

geofjamg commented Nov 9, 2023

geofjamg commented Nov 12, 2023

geofjamg Nov 23, 2023

caioluke Nov 23, 2023

annetill Nov 24, 2023

annetill commented Nov 24, 2023

annetill Nov 24, 2023

sonarqubecloud bot commented Nov 24, 2023

jeandemanged left a comment

Improve performance of incremental outer loops #895

Improve performance of incremental outer loops #895

Conversation

caioluke commented Nov 8, 2023 • edited by jeandemanged Loading

geofjamg commented Nov 9, 2023

jeandemanged commented Nov 9, 2023

geofjamg commented Nov 9, 2023

geofjamg commented Nov 12, 2023

geofjamg Nov 23, 2023

Choose a reason for hiding this comment

caioluke Nov 23, 2023

Choose a reason for hiding this comment

annetill Nov 24, 2023

Choose a reason for hiding this comment

annetill commented Nov 24, 2023

annetill Nov 24, 2023

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 24, 2023

jeandemanged left a comment

Choose a reason for hiding this comment

caioluke commented Nov 8, 2023 •

edited by jeandemanged

Loading