Current limiter implementation #340
Conversation
Signed-off-by: Hadrien <hadrien.godard@artelys.com>
Signed-off-by: Hadrien <hadrien.godard@artelys.com>
Add new tests Signed-off-by: Hadrien <hadrien.godard@artelys.com>
Signed-off-by: Hadrien <hadrien.godard@artelys.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
| @@ -130,4 +127,16 @@ public void onVoltageControlModeChange(DiscreteVoltageControl voltageControl, Di | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| @Override | |||
| public void onPhaseControlTapChange(DiscretePhaseControl phaseControl) { | |||
There was a problem hiding this comment.
This should be handled in the TargetVector to just update the alpha1 target value. There is no need to change the equation system. => performance will also be better by just updating the target (no need to rebuild the jacobian matrix)
There was a problem hiding this comment.
I have made a fix and I think it is correct. I knew that it was not the good solution when updating the equation system. So I have used the listeners you have implemented and I just want to say that I am impress by your work. It is really clever !
| @@ -146,4 +146,32 @@ public void roundR1ToClosestTap() { | |||
| } | |||
| r1 = Double.NaN; | |||
| } | |||
|
|
|||
| @Override | |||
| public boolean getNewTapPosition(Direction direction) { | |||
There was a problem hiding this comment.
This should be updateTapPosition. A getter is not suppose to change internal state.
This is here that the network notification should be fired regading tap position change.
=> so it means that PiModel should be part of Network listenable data.
| success = isSensibilityPositive ? piModel.getNewTapPosition(PiModel.Direction.DECREASE) : piModel.getNewTapPosition(PiModel.Direction.INCREASE); | ||
| } | ||
| if (success) { | ||
| for (LfNetworkListener listener : controllerBranch.getNetwork().getListeners()) { |
There was a problem hiding this comment.
Once notification will be managed inside PiModel we won't need this hack anymore because the call to getNewTapPosition will trigger the notif.
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
There was a problem hiding this comment.
Hi! Thanks for your work @Hadrien-Godard, I have changed many lines of code, because it was not the perfect way to implement the current limiter, but functionally it was correct. Many we have now to go deeper into the details and increase tests. Thanks for the current derivatives ! I am going to test this feature on real case.
geofjamg
changed the title
|
Kudos, SonarCloud Quality Gate passed! |
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 restWhat kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
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)