Resolve some TODOs (#1063)

data/crac-creation/crac-creator-cim/src/main/java/com/powsybl/openrao/data/craccreation/creator/cim/craccreator/NetworkActionCreator.java

@@ -84,7 +84,6 @@ public void createNetworkActionAdder() {
                 } else if (psrType.equals(CimConstants.PsrType.GENERATION.getStatus()) || psrType.equals(CimConstants.PsrType.LOAD.getStatus())) {
                     // Injection elementary action
                     addInjectionSetpointElementaryAction(elementaryActionId, remedialActionRegisteredResource, networkActionAdder);
-                    // ------ TODO : Missing check : default capacity in ohms
                 } else if (psrType.equals(CimConstants.PsrType.LINE.getStatus()) && remedialActionRegisteredResource.getMarketObjectStatusStatus().equals(CimConstants.MarketObjectStatus.ABSOLUTE.getStatus())) {
                     this.networkActionCreationContext = RemedialActionSeriesCreationContext.notImported(createdRemedialActionId, ImportStatus.NOT_YET_HANDLED_BY_OPEN_RAO, String.format("Modify line impedance as remedial action on elementary action %s", elementaryActionId));
                     return;

data/crac-creation/crac-creator-cse/src/main/java/com/powsybl/openrao/data/craccreation/creator/cse/CseCracCreator.java

@@ -74,7 +74,6 @@ CseCracCreationContext createCrac(CRACDocumentType cseCrac, Network network, Off
 
             creationContext.buildCreationReport();
             CracValidator.validateCrac(crac, network).forEach(creationContext.getCreationReport()::added);
-            // TODO : add unit test for CracValidator.validateCrac step when auto RAs are handled
             return creationContext.creationSuccess(crac);
         } catch (OpenRaoException e) {
             creationContext.getCreationReport().error(String.format("CRAC could not be created: %s", e.getMessage()));

data/crac-creation/crac-creator-cse/src/test/java/com/powsybl/openrao/data/craccreation/creator/cse/CseCracCreatorWithMneTest.java

@@ -82,8 +82,7 @@ private void assertMneNotImported(String name, ImportStatus importStatus) {
         assertFalse(cseCriticalBranchCreationContext.isSelected());
         assertFalse(cseCriticalBranchCreationContext.isImported());
         assertEquals(cseCriticalBranchCreationContext.getImportStatus(), importStatus);
-        assertNull(cseCriticalBranchCreationContext.getCreatedCnecsIds().get(name)); // TODO redo this assert ?
-        assertNull(cracCreationContext.getCrac().getFlowCnec(name));
+        assertNull(cseCriticalBranchCreationContext.getCreatedCnecsIds().get(name));
     }
 
     private void assertMneBaseCaseInCriticalBranchCreationContexts(String name, String fromNode, String toNode, String suffix, boolean inverted) {

data/crac/crac-api/src/main/java/com/powsybl/openrao/data/cracapi/CracFactory.java

@@ -67,7 +67,7 @@ static CracFactory find(String factoryName) {
             }
             throw new OpenRaoException("Crac factory '" + factoryName + "' not found");
         } else {
-            throw new OpenRaoException("No CracFactory implementation found, or no default implementation set and multiple implementation found.");
+            throw new OpenRaoException("No CracFactory implementation found, or no default implementation set and multiple implementations found.");
         }
     }
 

data/crac/crac-impl/src/main/java/com/powsybl/openrao/data/cracimpl/ContingencyAdderImpl.java

@@ -79,7 +79,5 @@ public Contingency add() {
             owner.addContingency(contingency);
             return owner.getContingency(id);
         }
-        // TODO : create additional states if there are RAs with "OnInstant" usage rule (on curative/auto instant)
-        // not required as as soon as there is no RA on AUTO instant
     }
 }

data/crac/crac-loopflow-extension/src/main/java/com/powsybl/openrao/data/cracloopflowextension/JsonCnecLoopFlowExtensionSerializer.java

@@ -6,19 +6,17 @@
  */
 package com.powsybl.openrao.data.cracloopflowextension;
 
-import com.powsybl.openrao.commons.OpenRaoException;
-import com.powsybl.openrao.commons.PhysicalParameter;
-import com.powsybl.openrao.commons.Unit;
-import com.powsybl.openrao.data.craciojson.ExtensionsHandler;
-import com.powsybl.openrao.data.cracapi.cnec.FlowCnec;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.google.auto.service.AutoService;
+import com.powsybl.openrao.commons.OpenRaoException;
+import com.powsybl.openrao.commons.Unit;
+import com.powsybl.openrao.data.cracapi.cnec.FlowCnec;
+import com.powsybl.openrao.data.craciojson.ExtensionsHandler;
 
 import java.io.IOException;
-import java.util.Objects;
 
 import static com.powsybl.openrao.data.craciojson.JsonSerializationConstants.deserializeUnit;
 import static com.powsybl.openrao.data.craciojson.JsonSerializationConstants.serializeUnit;
@@ -58,24 +56,7 @@ public LoopFlowThreshold deserialize(JsonParser jsonParser, DeserializationConte
                     throw new OpenRaoException("Unexpected field: " + jsonParser.getCurrentName());
             }
         }
-
-        // TODO : when constructor is replaced with adder, remove these checks
-        if (Double.isNaN(inputThreshold)) {
-            throw new OpenRaoException("Cannot add LoopFlowThreshold without a threshold value. Please use withValue() with a non null value");
-        }
-        if (Objects.isNull(unit)) {
-            throw new OpenRaoException("Cannot add LoopFlowThreshold without a threshold unit. Please use withUnit() with a non null value");
-        }
-        if (inputThreshold < 0) {
-            throw new OpenRaoException("LoopFlowThresholds must have a positive threshold.");
-        }
-        if (unit.equals(Unit.PERCENT_IMAX) && (inputThreshold > 1 || inputThreshold < 0)) {
-            throw new OpenRaoException("LoopFlowThresholds in Unit.PERCENT_IMAX must be defined between 0 and 1, where 1 = 100%.");
-        }
-        if (unit.getPhysicalParameter() != PhysicalParameter.FLOW) {
-            throw new OpenRaoException("LoopFlowThresholds can only be defined in AMPERE, MEGAWATT or PERCENT_IMAX");
-        }
-
+        LoopFlowThresholdUtils.checkAttributes(inputThreshold, unit);
         return new LoopFlowThresholdImpl(inputThreshold, unit);
     }
 

data/crac/crac-loopflow-extension/src/main/java/com/powsybl/openrao/data/cracloopflowextension/LoopFlowThresholdAdderImpl.java

@@ -6,13 +6,9 @@
  */
 package com.powsybl.openrao.data.cracloopflowextension;
 
-import com.powsybl.openrao.commons.OpenRaoException;
-import com.powsybl.openrao.commons.PhysicalParameter;
+import com.powsybl.commons.extensions.AbstractExtensionAdder;
 import com.powsybl.openrao.commons.Unit;
 import com.powsybl.openrao.data.cracapi.cnec.FlowCnec;
-import com.powsybl.commons.extensions.AbstractExtensionAdder;
-
-import java.util.Objects;
 
 /**
  * @author Baptiste Seguinot {@literal <baptiste.seguinot at rte-france.com>}
@@ -40,23 +36,7 @@ public LoopFlowThresholdAdder withUnit(Unit thresholdUnit) {
 
     @Override
     protected LoopFlowThreshold createExtension(FlowCnec flowCnec) {
-
-        if (Objects.isNull(thresholdValue)) {
-            throw new OpenRaoException("Cannot add LoopFlowThreshold without a threshold value. Please use withValue() with a non null value");
-        }
-        if (Objects.isNull(thresholdUnit)) {
-            throw new OpenRaoException("Cannot add LoopFlowThreshold without a threshold unit. Please use withUnit() with a non null value");
-        }
-        if (thresholdValue < 0) {
-            throw new OpenRaoException("LoopFlowThresholds must have a positive threshold.");
-        }
-        if (thresholdUnit.equals(Unit.PERCENT_IMAX) && (thresholdValue > 1 || thresholdValue < 0)) {
-            throw new OpenRaoException("LoopFlowThresholds in Unit.PERCENT_IMAX must be defined between 0 and 1, where 1 = 100%.");
-        }
-        if (thresholdUnit.getPhysicalParameter() != PhysicalParameter.FLOW) {
-            throw new OpenRaoException("LoopFlowThresholds can only be defined in AMPERE, MEGAWATT or PERCENT_IMAX");
-        }
-
+        LoopFlowThresholdUtils.checkAttributes(thresholdValue, thresholdUnit);
         return new LoopFlowThresholdImpl(thresholdValue, thresholdUnit);
     }
 }

data/crac/crac-loopflow-extension/src/main/java/com/powsybl/openrao/data/cracloopflowextension/LoopFlowThresholdImpl.java

@@ -30,9 +30,7 @@ of the branch current limit (retrieved from the Network)
     private double inputThreshold;
     private Unit inputThresholdUnit;
 
-    @Deprecated
-    //todo: make private package
-    public LoopFlowThresholdImpl(double value, Unit unit) {
+    LoopFlowThresholdImpl(double value, Unit unit) {
         this.inputThreshold = value;
         this.inputThresholdUnit = unit;
     }

data/crac/crac-loopflow-extension/src/main/java/com/powsybl/openrao/data/cracloopflowextension/LoopFlowThresholdUtils.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2024, RTE (http://www.rte-france.com)
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package com.powsybl.openrao.data.cracloopflowextension;
+
+import com.powsybl.openrao.commons.OpenRaoException;
+import com.powsybl.openrao.commons.PhysicalParameter;
+import com.powsybl.openrao.commons.Unit;
+
+/**
+ * @author Peter Mitri {@literal <peter.mitri at rte-france.com>}
+ */
+public final class LoopFlowThresholdUtils {
+    private LoopFlowThresholdUtils() {
+        // should not be used
+    }
+
+    static void checkAttributes(Double thresholdValue, Unit thresholdUnit) {
+        if (thresholdValue == null || Double.isNaN(thresholdValue)) {
+            throw new OpenRaoException("Cannot add LoopFlowThreshold without a threshold value. Please use withValue() with a non null value");
+        }
+        if (thresholdUnit == null) {
+            throw new OpenRaoException("Cannot add LoopFlowThreshold without a threshold unit. Please use withUnit() with a non null value");
+        }
+        if (thresholdValue < 0) {
+            throw new OpenRaoException("LoopFlowThresholds must have a positive threshold.");
+        }
+        if (thresholdUnit.equals(Unit.PERCENT_IMAX) && thresholdValue > 1) {
+            throw new OpenRaoException("LoopFlowThresholds in Unit.PERCENT_IMAX must be defined between 0 and 1, where 1 = 100%.");
+        }
+        if (thresholdUnit.getPhysicalParameter() != PhysicalParameter.FLOW) {
+            throw new OpenRaoException("LoopFlowThresholds can only be defined in AMPERE, MEGAWATT or PERCENT_IMAX");
+        }
+    }
+}

ra-optimisation/rao-api/src/main/java/com/powsybl/openrao/raoapi/parameters/LoadFlowAndSensitivityParameters.java

@@ -86,7 +86,7 @@ private SensitivityAnalysisParameters cleanLoadFlowParameters(SensitivityAnalysi
         loadFlowParameters.setWriteSlackBus(false);
         // in DC, as emulation AC is supported for LF but not for sensitivity analyses, it could
         // lead to incoherence.
-        if (loadFlowParameters.isDc()) {
+        if (loadFlowParameters.isDc() && loadFlowParameters.isHvdcAcEmulation()) {
             BUSINESS_WARNS.warn("The runs are in DC but the HvdcAcEmulation parameter is on: this is not compatible." +
                     "HvdcAcEmulation parameter set to false.");
             loadFlowParameters.setHvdcAcEmulation(false);

ra-optimisation/rao-api/src/test/java/com/powsybl/openrao/raoapi/RaoTest.java

@@ -8,6 +8,7 @@
 
 import com.powsybl.openrao.commons.OpenRaoException;
 import com.powsybl.openrao.data.cracapi.Crac;
+import com.powsybl.openrao.data.raoresultapi.ComputationStatus;
 import com.powsybl.openrao.data.raoresultapi.RaoResult;
 import com.powsybl.openrao.raoapi.parameters.RaoParameters;
 import com.powsybl.openrao.raoapi.raomock.AnotherRaoProviderMock;
@@ -65,10 +66,10 @@ void testDefaultOneProvider() {
         // run rao
         RaoResult result = defaultRao.run(raoInput, new RaoParameters());
         assertNotNull(result);
-        //todo : assertEquals(RaoResultImpl.Status.DEFAULT, result.getStatus());
+        assertEquals(ComputationStatus.DEFAULT, result.getComputationStatus());
         RaoResult resultAsync = defaultRao.runAsync(raoInput, new RaoParameters()).join();
         assertNotNull(resultAsync);
-        // todo: assertEquals(RaoResultImpl.Status.DEFAULT, resultAsync.getStatus());
+        assertEquals(ComputationStatus.DEFAULT, resultAsync.getComputationStatus());
     }
 
     @Test

ra-optimisation/search-tree-rao/src/main/java/com/powsybl/openrao/searchtreerao/castor/algorithm/CastorFullOptimization.java

@@ -177,7 +177,6 @@ public CompletableFuture<RaoResult> run() {
         boolean logFinalResultsOutsideOfSecondPreventive = true;
         // Run second preventive when necessary
         if (shouldRunSecondPreventiveRao(raoParameters, preventiveResult, postContingencyResults.values(), mergedRaoResults, targetEndInstant, preventiveRaoTime, raoInput.getCrac().getLastInstant())) {
-            // TODO: check if search tree has to be ran again
             RaoResult secondPreventiveRaoResults = runSecondPreventiveAndAutoRao(raoInput, raoParameters, stateTree, automatonTreeParameters, toolProvider, prePerimeterSensitivityAnalysis, initialOutput, preventiveResult, postContingencyResults);
             if (secondPreventiveImprovesResults(secondPreventiveRaoResults, mergedRaoResults)) {
                 mergedRaoResults = secondPreventiveRaoResults;

ra-optimisation/search-tree-rao/src/main/java/com/powsybl/openrao/searchtreerao/linearoptimisation/algorithms/fillers/CoreProblemFiller.java

@@ -201,7 +201,6 @@ private void addImpactOfRangeActionOnCnec(LinearProblem linearProblem, Sensitivi
         double sensitivity = sensitivityResult.getSensitivityValue(cnec, side, rangeAction, Unit.MEGAWATT);
 
         if (isRangeActionSensitivityAboveThreshold(rangeAction, Math.abs(sensitivity))) {
-            //TODO: compute this only once somehow
             double currentSetPoint = rangeActionActivationResult.getOptimizedSetpoint(rangeAction, state);
 
             // care : might not be robust as getCurrentValue get the current setPoint from a network variant

ra-optimisation/search-tree-rao/src/main/java/com/powsybl/openrao/searchtreerao/result/impl/OneStateOnlyRaoResultImpl.java

@@ -115,15 +115,13 @@ public double getPtdfZonalSum(Instant optimizedInstant, FlowCnec flowCnec, TwoSi
 
     public PerimeterResult getPerimeterResult(State state) {
         if (!state.equals(optimizedState)) {
-            // TODO : change this when getAppropriateResult will return a PerimeterResult (maybe throw an exception)
-            return null;
+            throw new OpenRaoException(WRONG_STATE);
         }
         return new PerimeterResultImpl(initialResult, postOptimizationResult);
     }
 
     public PerimeterResult getPostPreventivePerimeterResult() {
         if (!optimizedState.getInstant().isPreventive()) {
-            // TODO : review this also
             throw new OpenRaoException(WRONG_STATE);
         }
         return new PerimeterResultImpl(initialResult, postOptimizationResult);

ra-optimisation/search-tree-rao/src/main/java/com/powsybl/openrao/searchtreerao/result/impl/PerimeterResultImpl.java

@@ -125,10 +125,6 @@ public Set<RangeAction<?>> getActivatedRangeActions(State state) {
 
     @Override
     public double getOptimizedSetpoint(RangeAction<?> rangeAction, State state) {
-
-        // todo: check behaviour of this method when end of POC
-        // todo: move this logics in RangeActionActivationResultImpl (?)
-
         if (optimizationResult.getRangeActions().contains(rangeAction)) {
             return optimizationResult.getOptimizedSetpoint(rangeAction, state);
         }

ra-optimisation/search-tree-rao/src/main/java/com/powsybl/openrao/searchtreerao/searchtree/algorithms/SearchTree.java

@@ -237,7 +237,6 @@ private Object optimizeOneLeaf(AbstractNetworkPool networkPool, NetworkActionCom
                 } else {
                     // Apply range actions that have been changed by the previous leaf on the network to start next depth leaves
                     // from previous optimal leaf starting point
-                    // todo : Not sure previousDepthOptimalLeaf.getRangeActions() returns what we expect, this needs to be investigated
                     previousDepthOptimalLeaf.getRangeActions()
                         .forEach(ra ->
                             ra.apply(networkClone, previousDepthOptimalLeaf.getOptimizedSetpoint(ra, input.getOptimizationPerimeter().getMainOptimizationState()))

ra-optimisation/search-tree-rao/src/test/java/com/powsybl/openrao/searchtreerao/castor/algorithm/CastorFullOptimizationTest.java

@@ -764,7 +764,7 @@ void testThreeCurativeInstantsWithSecondCurativeHavingNoCnecAndNoRa() {
 
         assertEquals(671.88, raoResult.getFunctionalCost(null), 1.);
         assertEquals(429.22, raoResult.getFunctionalCost(crac.getInstant(InstantKind.PREVENTIVE)), 1.);
-        assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative1")), 1.); // TODO
+        assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative1")), 1.);
         assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative3")), 1.);
     }
 
@@ -871,8 +871,8 @@ void testThreeCurativeInstants() {
 
         assertEquals(671.88, raoResult.getFunctionalCost(null), 1.);
         assertEquals(429.22, raoResult.getFunctionalCost(crac.getInstant(InstantKind.PREVENTIVE)), 1.);
-        assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative1")), 1.); // TODO
-        assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative2")), 1.); // TODO
+        assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative1")), 1.);
+        assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative2")), 1.);
         assertEquals(-20.30, raoResult.getFunctionalCost(crac.getInstant("curative3")), 1.);
     }
 

ra-optimisation/search-tree-rao/src/test/java/com/powsybl/openrao/searchtreerao/commons/ToolProviderTest.java

@@ -80,11 +80,10 @@ void testCnecInCountry() {
         assertTrue(ToolProvider.cnecIsInCountryList(cnec1, network, Set.of(Country.BE, Country.DE)));
         assertFalse(ToolProvider.cnecIsInCountryList(cnec1, network, Set.of(Country.NL, Country.DE)));
 
-        // TODO : check this behavior
-        /*
-        assertTrue(ToolProvider.cnecIsInCountryList(cnec2, network, Set.of(Country.FR, Country.DE)));
-        assertTrue(ToolProvider.cnecIsInCountryList(cnec2, network, Set.of(Country.BE, Country.DE)));
-        assertTrue(ToolProvider.cnecIsInCountryList(cnec2, network, Set.of(Country.NL, Country.DE)));*/
+        // If country is empty, always return false
+        assertFalse(ToolProvider.cnecIsInCountryList(cnec2, network, Set.of(Country.FR, Country.DE)));
+        assertFalse(ToolProvider.cnecIsInCountryList(cnec2, network, Set.of(Country.BE, Country.DE)));
+        assertFalse(ToolProvider.cnecIsInCountryList(cnec2, network, Set.of(Country.NL, Country.DE)));
     }
 
     @Test

ra-optimisation/search-tree-rao/src/test/java/com/powsybl/openrao/searchtreerao/result/impl/AutomatonPerimeterResultImplTest.java

@@ -166,7 +166,6 @@ void testGetRangeActions() {
     }
 
     @Test
-    //TODO modify
     void testGetTapsAndSetpoints() {
         when(postAutoSensitivity.getSetpoint(pstRangeActionShifted)).thenReturn(rangeActionsWithSetpoint.get(pstRangeActionShifted));
         when(postAutoSensitivity.getSetpoint(unshiftedRangeAction)).thenReturn(rangeActionsWithSetpoint.get(unshiftedRangeAction));

ra-optimisation/search-tree-rao/src/test/java/com/powsybl/openrao/searchtreerao/result/impl/OneStateOnlyRaoResultImplTest.java

@@ -292,8 +292,9 @@ void testPreventiveCase() {
 
         // using another state
         State otherState = mock(State.class);
-        assertNull(output.getPerimeterResult(otherState));
-        OpenRaoException exception = assertThrows(OpenRaoException.class, () -> output.wasActivatedBeforeState(otherState, networkAction));
+        OpenRaoException exception = assertThrows(OpenRaoException.class, () -> output.getPerimeterResult(otherState));
+        assertEquals("Trying to access perimeter result for the wrong state.", exception.getMessage());
+        exception = assertThrows(OpenRaoException.class, () -> output.wasActivatedBeforeState(otherState, networkAction));
         assertEquals("Trying to access perimeter result for the wrong state.", exception.getMessage());
         exception = assertThrows(OpenRaoException.class, () -> output.isActivatedDuringState(otherState, networkAction));
         assertEquals("Trying to access perimeter result for the wrong state.", exception.getMessage());