Add bus id in voltage limit violation

EtienneLt commented 1 month ago

Please check if the PR fulfills these requirements

[X] 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? for voltage violation detection in Security Api, it returns the voltagelevelid

What is the new behavior (if this is a feature change)? for voltage violation detection in Security Api, returns bus id instead of voltagelevelid it will be desynchronised with olf security analysis

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

[x] Yes
[ ] No

If yes, please check if the following requirements are fulfilled

[ ] The Breaking Change or Deprecated label has been added
[ ] The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps) for voltage limit violation the detection in security will return bus id instead of voltage level id for the subject id

vidaldid-rte commented 3 weeks ago

I disagree that it is not a breaking change. It will change the behaviour for users of the security API since they receive a Bus instead of a Voltage Level. I think that the change is good. But

it needs to be visible in the documentation.
and in the release note.

EtienneLt commented 3 weeks ago

I updated the headers of the merge request

vidaldid-rte commented 3 weeks ago

One more thing about the decision to attach a violation to a bus: In a security analysis the list of bus in a contingence can be different from the list of bus in the N state. For example if a swtich is open in a contingence, two bus bars would then belong to different busses. So this change may produce weird results in node breaker topologies.

Looks like in node breaker, the only valid choice is to attach the violation:

To a Voltage Level but with a loss of information
To one or more BusBarSections

EtienneLt commented 3 weeks ago

One more thing about the decision to attach a violation to a bus: In a security analysis the list of bus in a contingence can be different from the list of bus in the N state. For example if a swtich is open in a contingence, two bus bars would then belong to different busses. So this change may produce weird results in node breaker topologies.

Looks like in node breaker, the only valid choice is to attach the violation: - To a Voltage Level but with a loss of information - To one or more BusBarSections

Yes but this pr will modify only for loadflow, for security analysis it is not defined yet because the buses on the variant are different and the idea is if there is a problem on a contingency a loadflow will be computed on the contingency

vidaldid-rte commented 3 weeks ago

Yes but this pr will modify only for loadflow, for security analysis it is not defined yet because the buses on the variant are different and the idea is if there is a problem on a contingency a loadflow will be computed on the contingency

Security.checkLimits calls LimitViolationDetection.checkAll that calls the modified checkVoltage that attaches the limits to a bus instead of a VL. So the change impacts security analysis. And it seems to me it may provide wrong results in node breaker topologies.

EtienneLt commented 3 weeks ago

Yes but this pr will modify only for loadflow, for security analysis it is not defined yet because the buses on the variant are different and the idea is if there is a problem on a contingency a loadflow will be computed on the contingency

Security.checkLimits calls LimitViolationDetection.checkAll that calls the modified checkVoltage that attaches the limits to a bus instead of a VL. So the change impacts security analysis. And it seems to me it may provide wrong results in node breaker topologies.

Is it a problem to have the wrong id for security analysis ? with the bus id we have the voltage level id so it can be found and if further information is needed, a modification on the network can be done and a loadflow can be computed

vidaldid-rte commented 3 weeks ago

Is it a problem to have the wrong id for security analysis ? with the bus id we have the voltage level id so it can be found and if further information is needed, a modification on the network can be done and a loadflow can be computed

For the log; as discussed, there is the possibility of adding an argument to distringuish between the N case where it is possible to use a bus, and the AS case where continuing to use a VL would never produce a wrong result.

rosiereflo commented 3 weeks ago

I fully agree with @vidaldid-rte remarks on this topic: returning the wrong id for SA is questionable. Moreover external simulation tools like DynaFlow will have hard time providing the proper data here.

sonarcloud[bot] commented 1 week ago

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

EtienneLt commented 1 week ago

rework in https://github.com/powsybl/powsybl-core/pull/3153

powsybl / powsybl-core

Add bus id in voltage limit violation #3134

Quality Gate failed