Connect transformers during generator connection #2989
Conversation
Signed-off-by: walameni <walha.ameni@gmail.com>
annetill
requested review from
flo-dup and
rolnico
and removed request for
colinepiloquet
iidm/iidm-modification/src/main/java/com/powsybl/iidm/modification/ConnectGenerator.java
Outdated
Show resolved
Hide resolved
iidm/iidm-modification/src/main/java/com/powsybl/iidm/modification/ConnectGenerator.java
Outdated
Show resolved
Hide resolved
iidm/iidm-modification/src/main/java/com/powsybl/iidm/modification/ConnectGenerator.java
Outdated
Show resolved
Hide resolved
iidm/iidm-modification/src/main/java/com/powsybl/iidm/modification/ConnectGenerator.java
Outdated
Show resolved
Hide resolved
iidm/iidm-modification/src/main/java/com/powsybl/iidm/modification/ConnectGenerator.java
Outdated
Show resolved
Hide resolved
iidm/iidm-modification/src/test/java/com/powsybl/iidm/modification/ConnectGeneratorTest.java
Outdated
Show resolved
Hide resolved
| new ConnectGenerator(g.getId()).apply(n); | ||
| if (!t.isConnected() || !g.getTerminal().getBusBreakerView().getConnectableBus().isInMainConnectedComponent()) { | ||
| //If the generator still disconnected, we should not change the active power setPoint | ||
| LOGGER.info("Generator {} could not be connected, discarded from scaling", g.getId()); | ||
| return 0.; | ||
| } | ||
|
|
There was a problem hiding this comment.
Please extract this part and the few surrounding lines in a new method to reduce the method complexity and avoid a brain overload issue
...idm-modification/src/main/java/com/powsybl/iidm/modification/scalable/GeneratorScalable.java
Outdated
Show resolved
Hide resolved
iidm/iidm-modification/src/test/java/com/powsybl/iidm/modification/ConnectGeneratorTest.java
Outdated
Show resolved
Hide resolved
iidm/iidm-modification/src/test/java/com/powsybl/iidm/modification/ConnectGeneratorTest.java
Outdated
Show resolved
Hide resolved
|
Additional question: you managed the TwoWindingsTransformers in this PR but should the ThreeWindingsTransformers be also reconnected the same way? |
Signed-off-by: walameni <walha.ameni@gmail.com>
|
| @@ -128,8 +128,12 @@ public double scale(Network n, double asked, ScalingParameters parameters) { | |||
| Terminal t = g.getTerminal(); | |||
| if (!t.isConnected()) { | |||
| if (parameters.isReconnect()) { | |||
| new ConnectGenerator(g.getId()).apply(n); | |||
| LOGGER.info("Connecting {}", g.getId()); | |||
| if (!connectGeneratorToMainComponent(n, g)) { | |||
There was a problem hiding this comment.
You have to extract more lines in order to reduce the method complexity, it is still at 18 (with a maximum allowed fixed at 15)
There was a problem hiding this comment.
The way of detecting the transformers which connect the generator to the network seem very specific to the usecase. It's not clear to me how to make it more generic, we could add the switch part I mention, but what about three-winding transformers, what if the generator is behind two (or more) transformers in line, should we connect all transformers as for the unit test case?
| if (!generator.getTerminal().getBusBreakerView().getConnectableBus().isInMainConnectedComponent()) { | ||
| revertInitialState(generator, transformers, twtConnectionInitialState); |
There was a problem hiding this comment.
We should avoid doing something and then revert it. In order to do so you could check beforehand if the two-winding transformers are connected to the main connected component.
| */ | ||
| private static void connectTransformersOfGenerator(Generator generator) { | ||
| Bus genBus = getBus(generator.getTerminal()); | ||
| Set<TwoWindingsTransformer> transformers = generator.getTerminal().getVoltageLevel().getTwoWindingsTransformerStream() |
There was a problem hiding this comment.
You don't fully check that the generator is connected to the network through transformers. Indeed,
- the generator might be connected to the network through a line AND also through one or several two-winding transformers; in that case you might be connecting the transformers whereas the generator is already connected to the main connected component
- the generator might be connected to the network through a two-winding transformer but with a retained breaker between the generator and the transformer
| transformers.forEach(twt -> { | ||
| LOGGER.info("Connecting twoWindingsTransformer {} linked to generator {}", twt.getId(), generator.getId()); | ||
| twtConnectionInitialState.put(twt.getId(), Pair.of(twt.getTerminal1().isConnected(), twt.getTerminal2().isConnected())); | ||
| twt.getTerminals().forEach(Terminal::connect); |
There was a problem hiding this comment.
Why connecting all transformers? one of them should be enough. Maybe we could add a parameter to the network modification to let the user decide.
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior (if this is a feature change)?
During the shift of GeneratorScalable, the generator should be connected before targetP update. Is some cases, the generator is connected to the main network with a transformer. We should connect this transformer so that the generator connection is effective and taken into account by loadflow computation.
If even connecting the transformer, the generator remains disconnected, we recommend reverting the modification and leaving the generator at targetP = 0. This way, the shift will be made with other scalable generators.
This feature solves a real case problem encountered when shifting Spain during capacity calculation process.
Does this PR introduce a breaking change or deprecate an API?
If yes, please check if the following requirements are fulfilled
What changes might users need to make in their application due to this PR? (migration steps)
Other information: