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

DcValueVoltageInitializer fails in presence of resistive only branches #683

Merged
merged 9 commits into from
Jan 6, 2023

Conversation

jeandemanged
Copy link
Member

Signed-off-by: Damien Jeandemange damien.jeandemange@artelys.com

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

What kind of change does this PR introduce?
Bug fix

What is the current behavior? (You can also link to an open issue here)
Exception thrown by DcValueVoltageInitializer if the newtwork contains a resistive only branch.

What is the new behavior (if this is a feature change)?
TBD

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

Other information:

java.lang.IllegalArgumentException: Branch 'l12' has reactance equal to zero

	at com.powsybl.openloadflow.dc.equations.AbstractClosedBranchDcFlowEquationTerm.<init>(AbstractClosedBranchDcFlowEquationTerm.java:42)
	at com.powsybl.openloadflow.dc.equations.ClosedBranchSide1DcFlowEquationTerm.<init>(ClosedBranchSide1DcFlowEquationTerm.java:24)
	at com.powsybl.openloadflow.dc.equations.ClosedBranchSide1DcFlowEquationTerm.create(ClosedBranchSide1DcFlowEquationTerm.java:33)
	at com.powsybl.openloadflow.dc.equations.DcEquationSystem.createImpedantBranch(DcEquationSystem.java:75)
	at com.powsybl.openloadflow.dc.equations.DcEquationSystem.createBranches(DcEquationSystem.java:116)
	at com.powsybl.openloadflow.dc.equations.DcEquationSystem.create(DcEquationSystem.java:126)
	at com.powsybl.openloadflow.dc.DcLoadFlowContext.getEquationSystem(DcLoadFlowContext.java:32)
	at com.powsybl.openloadflow.dc.DcLoadFlowEngine.run(DcLoadFlowEngine.java:65)
	at com.powsybl.openloadflow.dc.DcValueVoltageInitializer.prepare(DcValueVoltageInitializer.java:58)
	at com.powsybl.openloadflow.ac.DcValueVoltageInitializerTest.testFourBusNetworkResistiveOnlyBranch(DcValueVoltageInitializerTest.java:92)
``

jeandemanged and others added 2 commits December 28, 2022 09:54
Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@gmail.com>
@geofjamg
Copy link
Member

In fact, in addition to the regression from #666, there is also several issues that were already before:

  • spanning tree which is based in zero impedance criteria was only store for current dc mode, it should be available for both dc and ac mode in case of init DC + LF AC in order to have a consistent spanning tree with zero impedance booleans.
  • non impedance branches graph and spanning tree should be cached as created several times (especially at the end for flow calculation on zero impedance branches)

So I tried in this PR to store all zero impedance infos for both AC and DC and as much as possible set everything in a lazy way to only calculate what is needed.

if (minImpedance) {
for (LfBranch branch : branches) {
branch.setMinZ(dc, lowImpedanceThreshold);
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 should not do this here at a post loading fix. z cut could be done in a cleaner way at branch creation.

Copy link
Member

Choose a reason for hiding this comment

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

It has been designed like that because min z what dependent of the AC or DC context. Now, as we store both, we can do that at branch creation. Do you want me to do this?

@geofjamg geofjamg requested a review from annetill January 2, 2023 09:56
@geofjamg geofjamg marked this pull request as ready for review January 2, 2023 09:56
@geofjamg
Copy link
Member

geofjamg commented Jan 2, 2023

Instead of the boolean dc which is not very readable we could have an enum like ?

enum EquationMode {
    AC,
    DC_APPROX
}

@@ -75,6 +77,14 @@ public class LfNetwork extends AbstractPropertyBag implements PropertyBag {

private GraphConnectivity<LfBus, LfBranch> connectivity;

private Graph<LfBus, LfBranch> dcZeroImpedanceSubGraph;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe zeroImpedanceSubGraph and zeroImpedanceSpanningTree could be grouped in another object.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it could be a good idea.

Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 6, 2023

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.1% 95.1% Coverage
0.0% 0.0% Duplication

@annetill annetill merged commit 30f7825 into main Jan 6, 2023
@annetill annetill deleted the dc_v_initializer_r_only_branch branch January 6, 2023 14:41
caioluke pushed a commit that referenced this pull request Jan 10, 2023
#683)

Signed-off-by: Damien Jeandemange <damien.jeandemange@artelys.com>
Co-authored-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
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.

3 participants