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

Increase security analysis with temporary limits and violations filter #195

Merged
merged 36 commits into from
Mar 24, 2021

Conversation

annetill
Copy link
Member

@annetill annetill commented Jan 12, 2021

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, issue: #129

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Expected: current temporary limits.
  • Expected: a sort on limit violations. There is a rule not to detect violations in state N-1/K if they already exist in state N, expect if they are worst (violation of a limit that has a smaller acceptable duration). For a contingency and for a branch side, if several temporary limit are violated, only the worst one is displayed (the worst one means the smaller acceptable duration). The LimitViolationFilter is not used for the moment.

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:

  • The Breaking Change or Deprecated label has been added
  • The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:

(if any of the questions/checkboxes don't apply, please delete them entirely)

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>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@@ -21,6 +25,9 @@

private static final Logger LOGGER = LoggerFactory.getLogger(AbstractLfBranch.class);

private static final Comparator<CurrentLimits.TemporaryLimit> TEMPORARY_LIMITS_COMPARATOR =
Comparator.comparingDouble(CurrentLimits.TemporaryLimit::getValue).reversed();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering, isn't it the same to sort based on descending values as to sort based on ascending acceptableDuration? Then we wouldn't need to sort, as they are already sorted in TemporaryLimitImpl based on ascending acceptableDuration

@annetill
Copy link
Member Author

Now, we have to imagine a post-processing that, for a branch and for s side:

  • For the base case: keep the limit violation with the more important value ;
  • For the post-contingency case: keep the limit violation with the more important value and put it in the result only if not present in the base case.

SortedSet<LfTemporaryLimit> temporaryLimits = new TreeSet<>(TEMPORARY_LIMITS_COMPARATOR);
if (currentLimits != null) {
for (CurrentLimits.TemporaryLimit temporaryLimit : currentLimits.getTemporaryLimits()) {
double valuePerUnit = temporaryLimit.getValue() != Double.MAX_VALUE ? temporaryLimit.getValue() * bus.getNominalV() / PerUnit.SB : Double.MAX_VALUE;
Copy link
Member Author

Choose a reason for hiding this comment

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

Double.NaN?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed I think that we don't need to make the distinction between temporary and permanent limit. The permanent limit is just a temporary limit with infinite duration time. And with that, the comparator works for all limits and it will ease the post processing on limit violations.

tadam50 and others added 12 commits March 1, 2021 16:43
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
The temporary limits are already sorted

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@annetill annetill changed the title [WIP] Increase security analysis Increase security analysis Mar 12, 2021
@annetill annetill changed the title Increase security analysis Increase security analysis with temporary limits and violations filter Mar 12, 2021
if (currentLimits != null) {
double toPerUnit = bus.getNominalV() / PerUnit.SB;
for (LoadingLimits.TemporaryLimit temporaryLimit : currentLimits.getTemporaryLimits()) {
double valuePerUnit = temporaryLimit.getValue() != Double.MAX_VALUE ? temporaryLimit.getValue() * toPerUnit : Double.MAX_VALUE;
Copy link
Member Author

@annetill annetill Mar 12, 2021

Choose a reason for hiding this comment

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

Why the default value for limit value is Double.MAX_VALUE and not NaN?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because Double.NaN is not supported as value in the implementation TemporaryLimitAdder of powsybl-iidm-impl. Do you think we should still support a NaN value?

annetill and others added 2 commits March 12, 2021 17:40
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
@annetill annetill requested a review from geofjamg March 12, 2021 16:48
if (postContingencyComputationOk) {
detectViolations(
network.getBranches().stream().filter(b -> !lfContingency.getBranches().contains(b)),
network.getBuses().stream().filter(b -> !lfContingency.getBuses().contains(b)),
postContingencyLimitViolations);
}

preContingencyLimitViolations.forEach((subjectSideId, preContingencyViolation) -> {
LimitViolation postContingencyViolation = postContingencyLimitViolations.get(subjectSideId);
if (postContingencyViolation != null && postContingencyViolation.getAcceptableDuration() >= preContingencyViolation.getAcceptableDuration()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@geofjamg I think that the comparator should rely on getValue and not on getAcceptableDuration. What do you think ?

annetill and others added 6 commits March 16, 2021 15:27
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
…than 1.1.

Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
This reverts commit d369d07.

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup force-pushed the increase-security-analysis branch from 2450a99 to 9838e2a Compare March 22, 2021 10:14
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>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

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

94.2% 94.2% Coverage
0.0% 0.0% Duplication

@annetill annetill merged commit cb70cc1 into master Mar 24, 2021
@annetill annetill deleted the increase-security-analysis branch March 24, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants