-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve transformer voltage control AFTER_GNERATOR_VOLTAGE_CONTROL
#1032
Conversation
…overage of new code Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
1342166
to
e72da3b
Compare
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
…hooting Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a first rough diagonal review ... will look into it more in detail later on
src/main/java/com/powsybl/openloadflow/OpenLoadFlowParameters.java
Outdated
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/OpenLoadFlowParameters.java
Outdated
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/OpenLoadFlowParameters.java
Outdated
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/OpenLoadFlowParameters.java
Outdated
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/OpenLoadFlowParameters.java
Outdated
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/ac/outerloop/tap/GroupVoltageControlManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/ac/outerloop/tap/TransformerRatioManager.java
Show resolved
Hide resolved
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To start to talk!
src/main/java/com/powsybl/openloadflow/OpenLoadFlowParameters.java
Outdated
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/ac/outerloop/TransformerVoltageControlOuterLoop.java
Show resolved
Hide resolved
src/main/java/com/powsybl/openloadflow/ac/outerloop/tap/GroupVoltageControlManager.java
Outdated
Show resolved
Hide resolved
private double getMaxControlledNominalVoltage() { | ||
return maxControlledNominalVoltage; | ||
} | ||
private GroupVoltageControlManager groupVoltageControlManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that grouping transformer is not an option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to GeneratorVoltageControlManager. Some French jumped in the initial name.
} | ||
} | ||
|
||
private boolean isBusBehindVeryHighVoltageTransfo(LfBus bus, double limit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this method? If the controlled bus is higher that the nominal voltage limit, is it not enough? Because this part is really specific and maybe works only for our network.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be void
/** | ||
* @author Didier Vidal {@literal <didier.vidal-ext at rte-france.com>} | ||
*/ | ||
public class GroupVoltageControlManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you need a dedicated class for that, and with Group
inside the name: I don't see the link with group here. And all of this is not optional so for me it should be in TransformerVoltageControlOuterLoop
.
src/main/java/com/powsybl/openloadflow/ac/outerloop/tap/TransformerRatioManager.java
Show resolved
Hide resolved
I think this PR should be separated in two PRs:
|
…r name and enable json serde Signed-off-by: VIDAL Didier (Externe) <didier.vidal_externe@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
this.minNominalVoltageLimit = limitOverride < 0 ? calculateMaxControlledNominalVoltage(network) : limitOverride; | ||
} | ||
|
||
private static double calculateMaxControlledNominalVoltage(LfNetwork network) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a consistent name with the minNominalVoltageLimit and decide if it is a min or a max.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
-> computeDefaultMinNominalVoltageLimit
* Disables the voltage control of generators if controlled bus nominal voltage is under the limit. | ||
*/ | ||
public void disableGeneratorVoltageControlsUnderMaxControlledNominalVoltage(LfNetwork network) { | ||
for (LfBus bus : network.getControlledBuses(VoltageControl.Type.GENERATOR)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clear the disabledControllerBuses before ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I hesitated between a clear and an IllegalStateException if list not empty...
Opted for a clear with a comment in code that this use case of multiple calls was not the initial design intention and was not tested.
} | ||
} | ||
|
||
private boolean isBusBehindVeryHighVoltageTransfo(LfBus bus, double limit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=> isBusBehindStepupTransformer?
Indeed we can improve the check by
- checking that the branch is a transformer steping up the voltage
- multiple transformers starting from stator bus ?
src/main/java/com/powsybl/openloadflow/ac/outerloop/TransformerVoltageControlOuterLoop.java
Show resolved
Hide resolved
} | ||
} | ||
|
||
private final Map<String, Pair<Double, SharedControl>> sharedControlByBranchId = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using this Pair (not really readable) why not moving the r1 to SharedControl ? Just for using a record ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because as far as I understand a shared control is shared between the branches of a shared control, so the object is shared too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, right. So maybe we should have another class to embed the SharedControl and its r1 value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pair is often considered as a poor design choice and should try to avoid it in OLF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a record initially. Doesn't change much. What we need is a multikey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed I removed it because the good naming was not so easy.
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Didier Vidal <didier.vidal_externe@rte-france.com>
Signed-off-by: Didier Vidal <didier.vidal_externe@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Didier Vidal <didier.vidal_externe@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com> Signed-off-by: Didier Vidal <didier.vidal_externe@rte-france.com>
8a12f12
to
61e8a83
Compare
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Didier Vidal <didier.vidal_externe@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Didier Vidal <didier.vidal_externe@rte-france.com>
Quality Gate passedIssues Measures |
AFTER_GNERATOR_VOLTAGE_CONTROL
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
In order to provide a ratio tap change simulation that better matches our Network we introduce the following evolutions to the AfterVoltageControlOuterLoop:
The default value of the new parameter (transformerVoltageControlUseInitialTapPosition) keep a behavior close to the previous behavior of TransformerVoltageControlOuterLoop.
What is the current behavior?
What is the new behavior (if this is a feature change)?
With current default of new parameters, almost same as before.
If transformerVoltageControlStable is set to true, simulation much closer to actual behaviour of our network.
Does this PR introduce a breaking change or deprecate an API?
What changes might users need to make in their application due to this PR? (migration steps)