Skip to content
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

Remove transformer voltage controls when no PV buses at non controlled side #482

Merged
merged 21 commits into from
May 30, 2022

Conversation

annetill
Copy link
Member

@annetill annetill commented Mar 18, 2022

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest
Yes, fixes #481.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill annetill requested review from geofjamg and sylvlecl March 18, 2022 16:09
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill
Copy link
Member Author

AC sensitivities support is missing.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
if (componentOnNotControlledSide != null) {
Optional<LfBus> generatorControlledBus = componentOnNotControlledSide.stream().filter(LfBus::isVoltageControlled).findAny();
if (!generatorControlledBus.isPresent()) {
branch.setVoltageControlEnabled(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not forget to restore it in the outer loop cleanup!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is done as the condition is only to have a voltage control, no?

@geofjamg geofjamg changed the title Remove transformer voltage controls when no PV buses at non controlled side [WIP] Remove transformer voltage controls when no PV buses at non controlled side Apr 2, 2022
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>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill
Copy link
Member Author

For me: all is handled except in an AC sensitivity analysis, after a contingency, if a transformer becomes enable to control, the predefined results are not filled.

1 similar comment
@annetill
Copy link
Member Author

For me: all is handled except in an AC sensitivity analysis, after a contingency, if a transformer becomes enable to control, the predefined results are not filled.

…ontrolled

Signed-off-by: Hadrien <hadrien.godard@artelys.com>
@geofjamg
Copy link
Member

We should close this PR as when #522 will be done, we will only keep new the way the simulate tap changer (which is the only safe and correct way)

@sylvlecl
Copy link
Contributor

We should close this PR as when #522 will be done, we will only keep new the way the simulate tap changer (which is the only safe and correct way)

Won't you have KLU issues in sensitivities, in incremental mode, if you don't merge this one ?

Also, if the goal is to leave all modes, not only incremental, this one will be necessary. I am not sure it's a good idea to remove other modes, since I suspect the incremental mode will be very very slow ?

@annetill
Copy link
Member Author

We should close this PR as when #522 will be done, we will only keep new the way the simulate tap changer (which is the only safe and correct way)

Won't you have KLU issues in sensitivities, in incremental mode, if you don't merge this one ?

Also, if the goal is to leave all modes, not only incremental, this one will be necessary. I am not sure it's a good idea to remove other modes, since I suspect the incremental mode will be very very slow ?

I have tested the incremental mode on CGMES IOP files, and it is clearly a beta feature. I really need to spend more time on it, but I don't have this time for the moment. In that PR, @Hadrien-Godard and me are also working on voltage sensitivities. So we will continue.

@geofjamg
Copy link
Member

We should close this PR as when #522 will be done, we will only keep new the way the simulate tap changer (which is the only safe and correct way)

Won't you have KLU issues in sensitivities, in incremental mode, if you don't merge this one ?
Also, if the goal is to leave all modes, not only incremental, this one will be necessary. I am not sure it's a good idea to remove other modes, since I suspect the incremental mode will be very very slow ?

I have tested the incremental mode on CGMES IOP files, and it is clearly a beta feature. I really need to spend more time on it, but I don't have this time for the moment. In that PR, @Hadrien-Godard and me are also working on voltage sensitivities. So we will continue.

Every unrelated changes on voltage sensi have to move in another PR

@geofjamg
Copy link
Member

geofjamg commented May 17, 2022

We should close this PR as when #522 will be done, we will only keep new the way the simulate tap changer (which is the only safe and correct way)

Won't you have KLU issues in sensitivities, in incremental mode, if you don't merge this one ?

Also, if the goal is to leave all modes, not only incremental, this one will be necessary. I am not sure it's a good idea to remove other modes, since I suspect the incremental mode will be very very slow ?

Not significantly slower than with this mode.
Only one or two additional LU factorizations, in case we target directly the final tap.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill annetill changed the title [WIP] Remove transformer voltage controls when no PV buses at non controlled side Remove transformer voltage controls when no PV buses at non controlled side May 17, 2022
@sylvlecl
Copy link
Contributor

Not significantly slower than with this mode.
Only one or two additional LU factorizations, in case we target directly the final tap.

Ok, from the text of the PR I thought it was only +/-1 increments, but I understand it's not the case.

@sylvlecl
Copy link
Contributor

The previous discussion made me think that instead of the topological analysis, we could rely on a numerical analysis for the identification of those transfos.
Those would be the ones for which voltage sensitivity to the ratio is zero (epsilon).

Comment on lines 82 to 83
Optional<LfBus> generatorControlledBus = componentOnNotControlledSide.stream().filter(LfBus::isVoltageControlled).findAny();
if (!generatorControlledBus.isPresent()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can replace these two lines by if (componentOnNotControlledSide.stream().noneMatch(LfBus::isVoltageControlled)) {

@@ -47,4 +51,41 @@ protected OuterLoopStatus roundVoltageRatios(OuterLoopContext context) {
}
return status;
}

public static void checkControl(LfNetwork network) {
var connectivity = network.getConnectivity();
Copy link
Contributor

@flo-dup flo-dup May 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if using the EvenShiloachGraphDecrementalConnectivity here is consistent, as the init is costly.

If we launch a security analysis, that's not a question as the init will be reused.

But if we only launch a loadflow, that's not really appropriate, and the naive approach is 4 times faster.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we keep it this way, the performance issue has been fixed by #537. It was because we get the main connected component many times. And EvenShiloach algorithm recomputes the main connected component each time it is asked (it was first implemented for use cases where only the small components were needed).

Copy link
Member

@geofjamg geofjamg May 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if using the EvenShiloachGraphDecrementalConnectivity here is consistent, as the init is costly.

If we launch a security analysis, that's not a question as the init will be reused.

But if we only launch a loadflow, that's not really appropriate, and the naive approach is 4 times faster.

This piece of code is ran both for a simple LF and a SA. We probably should use naive implementation for the LF and EvenShiloach for the SA but in both cases it has to be fast so it's a good thing that you improved performance for EvenShiloach.

I also think that performance issue was more visible for the SA than simple LF.

geofjamg added 3 commits May 29, 2022 21:34
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
@annetill
Copy link
Member Author

Performances are great now!

annetill added 2 commits May 30, 2022 10:10
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
branch.getId());
disabledTransformerCount++;
}
if (componentOnNotControlledSide != null && componentOnNotControlledSide.stream().noneMatch(LfBus::isVoltageControlled)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance we're looping many times over a big set of LfBus? Then we should save the results for each component.
But it would mean that there is a big (main) connected component with many controller branches with the corresponding controlled buses outside that component. And no other controlled bus. Not very likely I suppose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's likely:
typically you will have a very large component for EHV (400-225 kV), on which will be connected most transformers that regulate on the lower level (90-63 kV in France).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a commit to save the result for each component, it's supposed to be a bit faster... or at least more robust. It seems that in practice it's not faster as in big components, the noneMatch encounters always quickly a controlled bus.

annetill and others added 2 commits May 30, 2022 12:17
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup force-pushed the fix-regleurs-enable-to-control branch from 503f0c4 to 42f5a70 Compare May 30, 2022 13:24
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

98.2% 98.2% Coverage
0.0% 0.0% Duplication

@flo-dup flo-dup merged commit 9b6203b into main May 30, 2022
@flo-dup flo-dup deleted the fix-regleurs-enable-to-control branch May 30, 2022 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Voltage controller tap changer leading to KLU issues
5 participants