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

Assert active power, reactive power and current consistency #545

Merged
merged 2 commits into from
Jun 7, 2022

Conversation

geofjamg
Copy link
Member

@geofjamg geofjamg commented Jun 5, 2022

Signed-off-by: Geoffroy Jamgotchian geoffroy.jamgotchian@gmail.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 ? 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, ...)
Unit test

What is the new behavior (if this is a feature change)?
A unit test has been added to assert p, q and i equations consistency

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>
@geofjamg geofjamg requested a review from annetill June 5, 2022 19:20
var i1Eq = new ClosedBranchSide1CurrentMagnitudeEquationTerm(branch, bus1, bus2, variableSet, true, true);
i1Eq.setStateVector(sv);
double i1 = i1Eq.eval();
assertEquals(i1, Math.hypot(p1, q1) / V_1, 10e-14);
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 is it normal to not have the sqrt(3) here ?

Copy link
Member

Choose a reason for hiding this comment

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

After looking at the code, yes it is normal. The sqrt(3) is managed by function PerUnit.ib() for sensitivity and security analysis.

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't mean it is correct. As far as I understand main principle of peruniting is to perunit only 2 quantities (for us it is apparent power and voltage) and all the other quantities are calculated from the 2 chosen ones.
So for me, the values in per unit are not correct even if one in SI are correct.

Copy link
Member Author

@geofjamg geofjamg Jun 7, 2022

Choose a reason for hiding this comment

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

According to https://en.wikipedia.org/wiki/Per-unit_system it seems correct

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 7, 2022

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@annetill annetill merged commit 35189bb into main Jun 7, 2022
@annetill annetill deleted the p_q_i_consistency branch June 7, 2022 11:29
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