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

Check target voltage plausibility on Transformers and Shunt Compensators #1030

Merged
merged 13 commits into from
May 17, 2024

Conversation

vmouradian
Copy link
Member

@vmouradian vmouradian commented May 15, 2024

Please check if the PR fulfills these requirements

  • 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?

No

What kind of change does this PR introduce?

Feature

What is the current behavior?

Target voltage plausibility is configured via minPlausibleTargetVoltage, maxPlausibleTargetVoltage and minNominalVoltageTargetVoltageCheck parameters.
Today the check is performed only for LfGenerators (Includes generators, batteries, SVCs, HVDC VSC converters).
No check is made for 1/ transformer ratio tap changers and 2/ shunt compensators.

What is the new behavior (if this is a feature change)?
Check is made also for 1/ transformer ratio tap changers and 2/ shunt compensators.
As an extra, now the number of disabled controls is reported (instead of only logged).

Does this PR introduce a breaking change or deprecate an API?

  • No

Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
@vmouradian vmouradian self-assigned this May 15, 2024
@vmouradian vmouradian changed the title BugFix : Check TargetV plausibility for voltage control on Transformers and Shunts [WIP] BugFix : Check TargetV plausibility for voltage control on Transformers and Shunts May 15, 2024
Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
@vmouradian vmouradian changed the title [WIP] BugFix : Check TargetV plausibility for voltage control on Transformers and Shunts BugFix : Check TargetV plausibility for voltage control on Transformers and Shunts May 15, 2024
@jeandemanged jeandemanged self-requested a review May 15, 2024 14:25
Copy link
Member

@jeandemanged jeandemanged left a comment

Choose a reason for hiding this comment

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

also:

  • LfNetworkLoadingReport to be enhanced with 2 new inconsistencies counters for RTCs and Shunts, similar to generatorsWithInconsistentTargetVoltage
  • warning log (see LfNetworkLoaderImpl#create)
  • maybe add reports for the 3 inconsistencies (generator, rtc, shunt) ?

@jeandemanged
Copy link
Member

@vmouradian let's also do some cleanup and remove linesWithDifferentNominalVoltageAtBothEnds from LfNetworkLoadingReport, this isn't used anymore (should have been removed in #658 but was missed)

vmouradian and others added 3 commits May 15, 2024 19:27
Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
if (!fixedShuntCompensators.isEmpty()) {
shunt = new LfShuntImpl(fixedShuntCompensators, network, this, false, parameters, topoConfig);
}
}
}

static boolean checkVoltageRegulation(ShuntCompensator shuntCompensator, LfNetworkParameters parameters, LfNetworkLoadingReport report) {
Copy link
Member

Choose a reason for hiding this comment

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

"Regulation" is never used in OLF, please use "Control" -> checkVoltageControl

Copy link
Member Author

Choose a reason for hiding this comment

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

Bad naming indeed, fixed in b43b026

Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
Copy link
Member

@jeandemanged jeandemanged left a comment

Choose a reason for hiding this comment

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

ok for main code so far for me
in test code, please keep existing tests unchanged, and define 3 new tests for the 3 checks, with 3 test reports txt.

Signed-off-by: vmouradian <valentin.mouradian@artelys.com>
@vmouradian
Copy link
Member Author

ok for main code so far for me in test code, please keep existing tests unchanged, and define 3 new tests for the 3 checks, with 3 test reports txt.

Adressed in 4026142

@jeandemanged jeandemanged changed the title BugFix : Check TargetV plausibility for voltage control on Transformers and Shunts Check target voltage plausibility on RatioTapChangers and ShuntCompensators May 16, 2024
@@ -843,15 +856,12 @@ private LfNetwork create(int numCC, int numSC, Network network, List<Bus> buses,
LOGGER.warn("Network {}: {} branches have been discarded because connected to same bus at both ends",
lfNetwork, report.branchesDiscardedBecauseConnectedToSameBusAtBothEnds);
}
if (report.linesWithDifferentNominalVoltageAtBothEnds > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

why it is removed?

Copy link
Member

Choose a reason for hiding this comment

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

this is dead code ... See #1030 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I think it is a mistake @geofjamg what's happen here?

Copy link
Member

Choose a reason for hiding this comment

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

return LfGenerator.isTargetVoltageNotPlausible(newTargetV, minPlausibleTargetVoltage, maxPlausibleTargetVoltage);
return !VoltageControl.isTargetVoltagePlausible(newTargetV, minPlausibleTargetVoltage, maxPlausibleTargetVoltage);
Copy link
Member

Choose a reason for hiding this comment

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

@annetill just a note, no check on minNominalVoltageTargetVoltageCheck here, I have no idea if this is deliberate (I lack knowledge about this outerloop).

Copy link
Member

Choose a reason for hiding this comment

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

I think we are going to wait for the issue @geofjamg . At RTE, we are using this outer loop on network without any generator connected to low nominal voltages, but it could arrived soon.

@@ -30,8 +30,6 @@ public class LfNetworkLoadingReport {

int branchesDiscardedBecauseConnectedToSameBusAtBothEnds = 0;

int linesWithDifferentNominalVoltageAtBothEnds = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Same question.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Copy link

sonarcloud bot commented May 17, 2024

Copy link
Member

@annetill annetill left a comment

Choose a reason for hiding this comment

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

Thanks!

@jeandemanged jeandemanged changed the title Check target voltage plausibility on RatioTapChangers and ShuntCompensators Check target voltage plausibility on Transformers and Shunt Compensators May 17, 2024
Copy link
Member

@jeandemanged jeandemanged left a comment

Choose a reason for hiding this comment

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

many thanks @vmouradian :)

@jeandemanged jeandemanged merged commit 4894ce3 into main May 17, 2024
6 checks passed
@jeandemanged jeandemanged deleted the bugfix/voltage-control-non-plausible-target-V branch May 17, 2024 12:19
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.

None yet

4 participants