Fix limits handling in slack distribution on generators #1041

jeandemanged · 2024-06-03T09:31:15Z

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?

No

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

We can observe a nasty "hysteresis" effect on active power slack distribution on generators.

Slack distribution on loads does not exhibit this behavior.

It is quite common on large networks that DistributedSlack outerloop first move generation in one direction, and then goes back in the other direction, in particular due to PV-PQ switching.
In the current implementation, the DistributedSlack outerloop always restart from the current generator targetP. Because of the effects of the active power limits, the solution becomes "solution path dependent". Ultimately, a perfectly initially balanced input network may end up having some of the generators being moved upward and other generators being moved downward.

Here the current behavior (click to expand)

  @Test
  void testSlackMismatchChangingSign() {
      parameters.setUseReactiveLimits(true).getExtension(OpenLoadFlowParameters.class).setSlackBusPMaxMismatch(0.0001);
      Network network = DistributedSlackNetworkFactory.createWithLossesAndPvPqTypeSwitch();
      Generator g1 = network.getGenerator("g1");
      Generator g2 = network.getGenerator("g2");
      Generator g3 = network.getGenerator("g3");
      Generator g4 = network.getGenerator("g4");

      parameters.setBalanceType(LoadFlowParameters.BalanceType.PROPORTIONAL_TO_GENERATION_PARTICIPATION_FACTOR);
      network.getGeneratorStream().map(g -> (ActivePowerControl) g.getExtension(ActivePowerControl.class)).forEach(e -> e.setParticipationFactor(1.0));
      g1.setMaxP(110.0);
      g3.setMaxP(110.0);
      g4.setMaxP(110.0);
      LoadFlowResult result = loadFlowRunner.run(network, parameters);
      assertTrue(result.isFullyConverged());

      var expectedDistributedActivePower = -network.getGeneratorStream().mapToDouble(g -> g.getTargetP() + g.getTerminal().getP()).sum();
      assertEquals(120.1988, expectedDistributedActivePower, LoadFlowAssert.DELTA_POWER);
      assertEquals(expectedDistributedActivePower, result.getComponentResults().get(0).getDistributedActivePower(), LoadFlowAssert.DELTA_POWER);

      // All generators have the same participation factor, and should increase generation by 120.1988 MW
      // generator | targetP | maxP
      // ----------|---------|-------
      //   g1      |  100    |  110  --> expected to hit limit 110MW with 10MW distributed
      //   g2      |   90    |  300  --> expected to pick up the remaining slack 70.1988 MW
      //   g3      |   90    |  110  --> expected to hit limit 110MW with 20MW distributed
      //   g4      |   90    |  110  --> expected to hit limit 110MW with 20MW distributed
      assertActivePowerEquals(-106.931, g1.getTerminal()); // FIXME should be -110
      assertActivePowerEquals(-279.404, g2.getTerminal()); // FIXME should be -270.1988
      assertActivePowerEquals(-106.931, g3.getTerminal()); // FIXME should be -110
      assertActivePowerEquals(-106.931, g4.getTerminal()); // FIXME should be -110
  }

What is the new behavior (if this is a feature change)?
Active power distribution on generators now always restarts from the same initial point, as defined by LfGenerator.initialTargetP, solving the issue.

Here the new fixed behavior (click to expand)

  @Test
  void testSlackMismatchChangingSign() {
      parameters.setUseReactiveLimits(true).getExtension(OpenLoadFlowParameters.class).setSlackBusPMaxMismatch(0.0001);
      network = DistributedSlackNetworkFactory.createWithLossesAndPvPqTypeSwitch();
      g1 = network.getGenerator("g1");
      g2 = network.getGenerator("g2");
      g3 = network.getGenerator("g3");
      g4 = network.getGenerator("g4");

      parameters.setBalanceType(LoadFlowParameters.BalanceType.PROPORTIONAL_TO_GENERATION_PARTICIPATION_FACTOR);
      for (var g : network.getGenerators()) {
          ActivePowerControl<Generator> ext = g.getExtension(ActivePowerControl.class);
          ext.setParticipationFactor(1.0);
      }

      g1.setMaxP(110.0);
      g3.setMaxP(110.0);
      g4.setMaxP(110.0);
      LoadFlowResult result = loadFlowRunner.run(network, parameters);
      assertTrue(result.isFullyConverged());

      var expectedDistributedActivePower = -network.getGeneratorStream().mapToDouble(g -> g.getTargetP() + g.getTerminal().getP()).sum();
      assertEquals(120.1976, expectedDistributedActivePower, LoadFlowAssert.DELTA_POWER);
      assertEquals(expectedDistributedActivePower, result.getComponentResults().get(0).getDistributedActivePower(), LoadFlowAssert.DELTA_POWER);

      // All generators have the same participation factor, and should increase generation by 120.1976 MW
      // generator | targetP | maxP
      // ----------|---------|-------
      //   g1      |  100    |  110  --> expected to hit limit 110MW with 10MW distributed
      //   g2      |   90    |  300  --> expected to pick up the remaining slack 70.1976 MW
      //   g3      |   90    |  110  --> expected to hit limit 110MW with 20MW distributed
      //   g4      |   90    |  110  --> expected to hit limit 110MW with 20MW distributed
      assertActivePowerEquals(-110.000, g1.getTerminal());
      assertActivePowerEquals(-270.1976, g2.getTerminal());
      assertActivePowerEquals(-110.000, g3.getTerminal());
      assertActivePowerEquals(-110.000, g4.getTerminal());
  }

In the context of security analysis, still the previous behavior needs to be maintained, this was done as follows:

after computing N state (a.k.a. basecase state, a.k.a. pre-contingency state), generators' initial target P are set to solved targetP, doing this sets a new "starting point" for slack distribution for post-contingency states. Doing this maintains previous behavior.
similarly, after computing N-k states (a.k.a. post-contingency state), generators' initial target P are set to solved targetP, doing this sets a new "starting point" for slack distribution for operator strategy states. Same, doing this maintains previous behavior.
regarding Generators' actions (as part of Operator Strategies): the action is set to modify not only the targetP but also the initialTargetP. Same, doing this maintains previous behavior.

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

No
⚠️ but a behavioral change to be approved by the community

Extra Information
No existing unit test were harmed with this bugfix, in particular:

load flow slack distribution tests
security analysis with operator strategies
HVDC were up to now the only "users" of initialTargetP field

Signed-off-by: Damien Jeandemange <[email protected]>

jeandemanged · 2024-06-25T15:33:18Z

src/main/java/com/powsybl/openloadflow/ac/outerloop/DistributedSlackOuterLoop.java

+        double absMismatch = Math.abs(slackBusActivePowerMismatch);
+        boolean shouldDistributeSlack = absMismatch > slackBusPMaxMismatch / PerUnit.SB && absMismatch > ActivePowerDistribution.P_RESIDUE_EPS;


ℹ️ Note to reviewers: This part could be a separate BugFix PR... let me know...

if slackBusPMaxMismatch is small in comparison with ActivePowerDistribution.P_RESIDUE_EPS, the outerloop becomes unstable because 1/ ActivePowerDistribution will not distribute anything and 2/ Outerloop says something has to be distributed

Thank you for catching this! I think we can let it in this PR.

jeandemanged · 2024-06-25T16:24:35Z

src/main/java/com/powsybl/openloadflow/network/util/GenerationActivePowerDistributionStep.java

+        double mismatch = remainingMismatch;
+        if (iteration == 0) {
+            // "undo" everything from targetP to go back to initialP
+            for (ParticipatingElement participatingGenerator : participatingElements) {
+                LfGenerator generator = (LfGenerator) participatingGenerator.getElement();
+                done += generator.getInitialTargetP() - generator.getTargetP();
+                mismatch -= generator.getInitialTargetP() - generator.getTargetP();
+                generator.setTargetP(generator.getInitialTargetP());
+            }
+        }


ℹ️ Note to reviewers

This is the way I found to fix the issue AND also avoiding a massive refactor: today ActivePowerDistribution is completely stateless. Making ActivePowerDistribution aware of a "starting point" would require more work / refactorings...

Note also that such refactoring's will need to be addressed for future developments planned in #979 and powsybl-entsoe/# - planned end of this year / Q4-2024 on our side.
Indeed, imagine a merit order rebalancing, the starting point is crucial, otherwise you would end up with both generators from upward list moved and generators from downward list moved ...

jeandemanged · 2024-06-25T16:31:57Z

@vmouradian I added you as reviewer because I noticed that this change if not properly implemented would break what you did in #1017 - unit tests are unchanged, but your double check is welcomed.

Hadrien-Godard · 2024-06-26T11:39:45Z

src/main/java/com/powsybl/openloadflow/network/util/ActivePowerDistribution.java

-                if (stepResult.movedBuses()) {
-                    movedBuses = true;
-                }
+                double done = step.run(participatingElements, iteration, remainingMismatch);


"powerDistributed" might be a clearer name than "done", don't you agree?

Hadrien-Godard · 2024-06-26T11:44:12Z

src/main/java/com/powsybl/openloadflow/network/util/GenerationActivePowerDistributionStep.java

    }

    @Override
-    public ActivePowerDistribution.StepResult run(List<ParticipatingElement> participatingElements, int iteration, double remainingMismatch) {
+    public double run(List<ParticipatingElement> participatingElements, int iteration, double remainingMismatch) {
        // normalize participation factors at each iteration start as some
        // generators might have reach a limit and have been discarded
        ParticipatingElement.normalizeParticipationFactors(participatingElements);

        double done = 0d;


Same, another name than "done" might be better (even if the name "done" was not introduced in this PR).

vmouradian · 2024-06-27T07:56:17Z

@vmouradian I added you as reviewer because I noticed that this change if not properly implemented would break what you did in #1017 - unit tests are unchanged, but your double check is welcomed.

I double checked for the SA generator action part, and everything seems indeed to work as expected

annetill · 2024-06-28T14:23:38Z

As a general comment, I think we have to use our summer time to add functional documentation about this kind of features. We can add new pages in the web-site about features and robustness. @jeandemanged What do you thing?

annetill · 2024-06-28T14:27:28Z

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

@@ -334,6 +334,7 @@ private void applyGeneratorChange(LfNetworkParameters networkParameters) {
        if (!generator.isDisabled()) {
            double newTargetP = generatorChange.isRelative() ? generator.getTargetP() + generatorChange.activePowerValue() : generatorChange.activePowerValue();
            generator.setTargetP(newTargetP);


I was about to merge the PR but I have a doubt, why you don't change these lines too in a LfContingency:

Set<LfBus> generatorBuses = new HashSet<>(); for (LfGenerator generator : lostGenerators) { **generator.setTargetP(0);** -> initial targetP to zero too ? LfBus bus = generator.getBus(); generatorBuses.add(bus); generator.setParticipating(false); generator.setDisabled(true);

Checked: indeed the generator.setParticipating(false); solved this issue.

indeed, but adding setInitialTargetP(0) would improve code clarity and won't harm, I am adding it.

Signed-off-by: Damien Jeandemange <[email protected]>

sonarqubecloud · 2024-07-01T07:45:48Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jeandemanged added 4 commits June 3, 2024 11:08

repro issue

271fac8

Signed-off-by: Damien Jeandemange <[email protected]>

Merge branch 'main' into fix-slack-distrib

c1cc537

fix

0b9362a

Signed-off-by: Damien Jeandemange <[email protected]>

clean

f1a7faa

Signed-off-by: Damien Jeandemange <[email protected]>

jeandemanged commented Jun 25, 2024

View reviewed changes

jeandemanged changed the title ~~[WIP] Fix limits handling in slack distribution on generators~~ Fix limits handling in slack distribution on generators Jun 25, 2024

jeandemanged added bug Something isn't working ready-for-review labels Jun 25, 2024

jeandemanged requested review from Hadrien-Godard and vmouradian June 25, 2024 16:07

jeandemanged commented Jun 25, 2024

View reviewed changes

Hadrien-Godard reviewed Jun 26, 2024

View reviewed changes

Hadrien-Godard approved these changes Jun 26, 2024

View reviewed changes

annetill added the PR: next-release label Jun 27, 2024

geofjamg approved these changes Jun 28, 2024

View reviewed changes

Merge branch 'main' into fix-slack-distrib

ac15815

annetill reviewed Jun 28, 2024

View reviewed changes

fix

f7d727e

Signed-off-by: Damien Jeandemange <[email protected]>

annetill approved these changes Jul 1, 2024

View reviewed changes

annetill merged commit a143c2b into main Jul 1, 2024
7 checks passed

annetill deleted the fix-slack-distrib branch July 1, 2024 07:57

jeandemanged mentioned this pull request Jul 17, 2024

Security Analysis: Fix Generator initialTargetP in OperatorStrategies #1069

Merged

3 tasks

jeandemanged mentioned this pull request Sep 4, 2024

Fix slack distribution when reference generator is not participating element #1084

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix limits handling in slack distribution on generators #1041

Fix limits handling in slack distribution on generators #1041

jeandemanged commented Jun 3, 2024 •

edited

Loading

jeandemanged Jun 25, 2024 •

edited

Loading

Hadrien-Godard Jun 26, 2024

jeandemanged Jun 25, 2024 •

edited

Loading

jeandemanged commented Jun 25, 2024

Hadrien-Godard Jun 26, 2024

Hadrien-Godard Jun 26, 2024

vmouradian commented Jun 27, 2024

annetill commented Jun 28, 2024

annetill Jun 28, 2024

annetill Jul 1, 2024

jeandemanged Jul 1, 2024 •

edited

Loading

sonarqubecloud bot commented Jul 1, 2024

		double absMismatch = Math.abs(slackBusActivePowerMismatch);
		boolean shouldDistributeSlack = absMismatch > slackBusPMaxMismatch / PerUnit.SB && absMismatch > ActivePowerDistribution.P_RESIDUE_EPS;

Fix limits handling in slack distribution on generators #1041

Fix limits handling in slack distribution on generators #1041

Conversation

jeandemanged commented Jun 3, 2024 • edited Loading

jeandemanged Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Hadrien-Godard Jun 26, 2024

Choose a reason for hiding this comment

jeandemanged Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

jeandemanged commented Jun 25, 2024

Hadrien-Godard Jun 26, 2024

Choose a reason for hiding this comment

Hadrien-Godard Jun 26, 2024

Choose a reason for hiding this comment

vmouradian commented Jun 27, 2024

annetill commented Jun 28, 2024

annetill Jun 28, 2024

Choose a reason for hiding this comment

annetill Jul 1, 2024

Choose a reason for hiding this comment

jeandemanged Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Jul 1, 2024

Quality Gate passed

jeandemanged commented Jun 3, 2024 •

edited

Loading

jeandemanged Jun 25, 2024 •

edited

Loading

jeandemanged Jun 25, 2024 •

edited

Loading

jeandemanged Jul 1, 2024 •

edited

Loading