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 EquationUtil #447

Merged
merged 18 commits into from
Feb 23, 2022
Merged

Remove EquationUtil #447

merged 18 commits into from
Feb 23, 2022

Conversation

geofjamg
Copy link
Member

@geofjamg geofjamg commented Feb 7, 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
No

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

What is the current behavior? (You can also link to an open issue here)
The EquationUtil allows to save and restore equations state change because of a contingency.

What is the new behavior (if this is a feature change)?
We don't use EquationUtil anymore but only relies on bus/branch disabling. It means that there is no more direct access to equation system from LF/AS/Sensi. We always interact with the LfNetwork which automatically reflects changes to the equation system.

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: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
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>
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>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
@geofjamg geofjamg changed the title Remove EquationUtil [WIP] Remove EquationUtil Feb 7, 2022
@geofjamg geofjamg requested a review from annetill February 7, 2022 08:07
# Conflicts:
#	src/main/java/com/powsybl/openloadflow/network/BusState.java
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
@@ -112,6 +109,9 @@ private void calculatePostContingencySensitivityValues(List<LfSensitivityFactor<
LoadFlowParameters lfParameters, OpenLoadFlowParameters lfParametersExt,
String contingencyId, int contingencyIndex, SensitivityValueWriter valueWriter,
Reporter reporter, boolean hasTransformerBusTargetVoltage) {
for (LfBranch branch : lfContingency.getBranches()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@annetill we can safely replace by LfContingency.apply ?

Copy link
Member

Choose a reason for hiding this comment

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

It is on my TODO list, I think it should work but I need to add some tests and to perform real tests. My question is: in an AC sensitivity analysis, do you need to support transformers control such as voltage or active power control ? Do we need to support shunt voltage control ?

Copy link
Member Author

Choose a reason for hiding this comment

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

For a sensitivity analysis, everything regarding control does not make really sense.

@geofjamg geofjamg changed the title [WIP] Remove EquationUtil Remove EquationUtil Feb 15, 2022
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
# Conflicts:
#	src/main/java/com/powsybl/openloadflow/sensi/AcSensitivityAnalysis.java
equationSystem.getEquation(bus.getNum(), AcEquationType.BUS_TARGET_V)
.orElseThrow()
.setActive(false);
bus.getVoltageControl().ifPresent(voltageControl -> AcEquationSystem.updateGeneratorVoltageControl(voltageControl, equationSystem));
Copy link
Member

Choose a reason for hiding this comment

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

Do you miss controllerBus.getReactivePowerControl().ifPresent here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, fixed

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
@geofjamg geofjamg requested a review from annetill February 23, 2022 11:52
@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

95.2% 95.2% Coverage
0.0% 0.0% Duplication

@annetill annetill merged commit b01b642 into main Feb 23, 2022
@annetill annetill deleted the remove_equation_util2 branch February 23, 2022 13:37
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.

2 participants