review feedback

Signed-off-by: Didier Vidal <didier.vidal_externe@rte-france.com>

docs/grid_model/extensions.md

@@ -31,7 +31,7 @@ generator.newExtension(ActivePowerControlAdder.class)
     .withParticipationFactor(1.5)
     .add();
 ```
-
+If defined, min targetP and max targetP must be in the [pMin, pMax] interval of the Generator or Battery.
 The participation status, the participation factor, the max targetP and the min targetP are multi-variants: they can vary from one variant to another.
 
 This extension is provided by the `com.powsybl:powsybl-iidm-extensions` module.

iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/extensions/ActivePowerControlImpl.java

@@ -7,13 +7,16 @@
  */
 package com.powsybl.iidm.network.impl.extensions;
 
+import com.powsybl.commons.PowsyblException;
 import com.powsybl.commons.util.trove.TBooleanArrayList;
 import com.powsybl.iidm.network.Battery;
 import com.powsybl.iidm.network.Generator;
 import com.powsybl.iidm.network.Injection;
 import com.powsybl.iidm.network.extensions.ActivePowerControl;
 import com.powsybl.iidm.network.impl.AbstractMultiVariantIdentifiableExtension;
 import gnu.trove.list.array.TDoubleArrayList;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.List;
 import java.util.OptionalDouble;
@@ -24,6 +27,7 @@
 public class ActivePowerControlImpl<T extends Injection<T>> extends AbstractMultiVariantIdentifiableExtension<T>
         implements ActivePowerControl<T> {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(ActivePowerControlImpl.class);
     private final TBooleanArrayList participate;
 
     private final TDoubleArrayList droop;
@@ -65,8 +69,9 @@ public ActivePowerControlImpl(T component,
         checkTargetPLimits(minTargetP, maxTargetP);
     }
 
-    private double checkTargetPLimit(double targetPLimit, String name, T injection) {
+    record PLimits(double minP, double maxP) { }
 
+    private PLimits getPLimits(T injection) {
         double maxP = Double.MAX_VALUE;
         double minP = -Double.MAX_VALUE;
         if (injection instanceof Generator) {
@@ -77,9 +82,24 @@ private double checkTargetPLimit(double targetPLimit, String name, T injection)
             maxP = ((Battery) injection).getMaxP();
             minP = ((Battery) injection).getMinP();
         }
+        return new PLimits(minP, maxP);
+    }
+
+    private double withinPMinMax(double value, T injection) {
+        PLimits pLimits = getPLimits(injection);
+
+        if (!Double.isNaN(value) && (value < pLimits.minP || value > pLimits.maxP)) {
+            LOGGER.warn("targetP limit is now outside of pMin,pMax for component " + injection.getId() + ". Returning closest value in [pmin,pMax].");
+            return value < pLimits.minP ? pLimits.minP : pLimits.maxP;
+        }
+        return value;
+    }
+
+    private double checkTargetPLimit(double targetPLimit, String name, T injection) {
+        PLimits pLimits = getPLimits(injection);
 
-        if (!Double.isNaN(targetPLimit) && (targetPLimit < minP || targetPLimit > maxP)) {
-            throw new IllegalArgumentException(String.format("%s value (%s) is no between minP and maxP for component %s",
+        if (!Double.isNaN(targetPLimit) && (targetPLimit < pLimits.minP || targetPLimit > pLimits.maxP)) {
+            throw new PowsyblException(String.format("%s value (%s) is not between minP and maxP for component %s",
                     name,
                     targetPLimit,
                     injection.getId()));
@@ -91,7 +111,7 @@ private double checkTargetPLimit(double targetPLimit, String name, T injection)
     private void checkTargetPLimits(double minTargetP, double maxTargetP) {
         if (!Double.isNaN(minTargetP) && !Double.isNaN(maxTargetP)) {
             if (minTargetP > maxTargetP) {
-                throw new IllegalArgumentException("invalid targetP limits [" + minTargetP + ", " + maxTargetP + "]");
+                throw new PowsyblException("invalid targetP limits [" + minTargetP + ", " + maxTargetP + "]");
             }
         }
     }
@@ -152,7 +172,7 @@ public void allocateVariantArrayElement(int[] indexes, int sourceIndex) {
     @Override
     public OptionalDouble getMinTargetP() {
         double result = minTargetP.get(getVariantIndex());
-        return Double.isNaN(result) ? OptionalDouble.empty() : OptionalDouble.of(result);
+        return Double.isNaN(result) ? OptionalDouble.empty() : OptionalDouble.of(withinPMinMax(result, getExtendable()));
     }
 
     @Override
@@ -164,7 +184,7 @@ public void setMinTargetP(double minTargetP) {
     @Override
     public OptionalDouble getMaxTargetP() {
         double result = maxTargetP.get(getVariantIndex());
-        return Double.isNaN(result) ? OptionalDouble.empty() : OptionalDouble.of(result);
+        return Double.isNaN(result) ? OptionalDouble.empty() : OptionalDouble.of(withinPMinMax(result, getExtendable()));
     }
 
     @Override

iidm/iidm-tck/src/test/java/com/powsybl/iidm/network/tck/extensions/AbstractActivePowerControlTest.java

@@ -19,6 +19,7 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.OptionalDouble;
 
 import static com.powsybl.iidm.network.VariantManagerConstants.INITIAL_VARIANT_ID;
 import static org.junit.jupiter.api.Assertions.*;
@@ -101,6 +102,12 @@ public void variantsCloneTest() {
         assertThrows(IllegalArgumentException.class, () -> activePowerControl.setMinTargetP(201));
         assertThrows(IllegalArgumentException.class, () -> activePowerControl.setMaxTargetP(99));
 
+        // try to fool the extension
+        bat.setMaxP(190);
+        bat.setMinP(110);
+        assertEquals(OptionalDouble.of(190), activePowerControl.getMaxTargetP());
+        assertEquals(OptionalDouble.of(110), activePowerControl.getMinTargetP());
+
         // Test removing current variant
         variantManager.removeVariant(variant3);
         try {