Operator strategy simulation fixes #633
Conversation
geofjamg
changed the title
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
geofjamg
changed the title
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
| @@ -181,10 +202,4 @@ public void apply() { | |||
| branch.getPiModel().setTapPosition(newTapPosition); | |||
| } | |||
| } | |||
|
|
|||
| private static void checkBranch(LfBranch branch, String branchId) { | |||
There was a problem hiding this comment.
So, in that case, no log to the user? And ok no exception.
There was a problem hiding this comment.
I removed the check at lfnetwork level but it would be better to keep it on iidm network
There was a problem hiding this comment.
There is nothing at iidm network I think.
There was a problem hiding this comment.
Yes i will add it in this pr
There was a problem hiding this comment.
I added to check of actions at IIDM network level and at LfNetwork level we can suppose that if not found it is in another component. what are we supposed to do in that case? Maybe we new status added in next core release, we can have the status "no impact" for this operator strategy result?
Also for IIDM check, it is relevant to throw an exception if an action does not exist in the network ? this is what we are doing for contingencies but maybe we should only warm the user that this contingency or action is invalid and discarded?
geofjamg
changed the title
annetill
changed the title
# Conflicts: # src/test/java/com/powsybl/openloadflow/sa/LfActionTest.java
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
geofjamg
changed the title
|
Kudos, SonarCloud Quality Gate passed! |
|
@annetill it is now ready for review |
| List<OperatorStrategy> operatorStrategies) { | ||
| List<OperatorStrategy> operatorStrategies, | ||
| Map<String, Action> actionsById, | ||
| Set<Action> neededActions) { |
There was a problem hiding this comment.
What I understand from previous talk with @sylvlecl and you is that the actions order matters. That is why I used a list instead of a set. With your modifications, are we sure that order is conserved?
| Set<String> contingencyIds = propagatedContingencies.stream().map(propagatedContingency -> propagatedContingency.getContingency().getId()).collect(Collectors.toSet()); | ||
| Map<String, List<OperatorStrategy>> operatorStrategiesByContingencyId = new HashMap<>(); | ||
| for (OperatorStrategy operatorStrategy : operatorStrategies) { | ||
| if (contingencyIds.contains(operatorStrategy.getContingencyId())) { | ||
| operatorStrategiesByContingencyId.computeIfAbsent(operatorStrategy.getContingencyId(), key -> new ArrayList<>()).add(operatorStrategy); | ||
| // check actions IDs exists | ||
| for (String actionId : operatorStrategy.getActionIds()) { |
There was a problem hiding this comment.
@annetill this is here that the order is defined and it is a List
Signed-off-by: Geoffroy Jamgotchian geoffroy.jamgotchian@rte-france.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)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'and skip the restNo
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix
What is the current behavior? (You can also link to an open issue here)
2 bugs:
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:
Other information:
(if any of the questions/checkboxes don't apply, please delete them entirely)