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

Use new parameters isDcUseTransformerRatio and countriesToBalance #308

Merged
merged 8 commits into from
May 21, 2021

Conversation

obrix
Copy link
Member

@obrix obrix commented May 20, 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)

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

Use the two new parameters available in powsybl-core isDcUseTransformerRatio and countriesToBalance. The branch for this PR is based on the branch from the PR #295 to use the compatibility work already done for 4.2.0 release candidate.
Thus it is to be rebased and merged after this PR.

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)

@obrix obrix requested a review from flo-dup May 20, 2021 11:24
@annetill annetill changed the base branch from master to handle_several_cc_networks May 20, 2021 11:41
Base automatically changed from handle_several_cc_networks to master May 20, 2021 12:08
…triesToBalance.

Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@obrix obrix closed this May 20, 2021
@obrix obrix deleted the lf_new_parameters branch May 20, 2021 12:24
@obrix obrix reopened this May 20, 2021
@obrix obrix force-pushed the lf_new_parameters branch from 7650725 to cc718eb Compare May 20, 2021 12:31
obrix added 3 commits May 20, 2021 14:48
…articipating to the computation based on its substation country.

Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
…o update debug-parameters.json

Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@obrix obrix requested a review from annetill May 20, 2021 13:32
@obrix obrix changed the title [WIP] Use new parameters isDcUseTransformerRatio and countriesToBalance Use new parameters isDcUseTransformerRatio and countriesToBalance May 20, 2021
Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

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

In the end, that's not at all because of this PR, but these dc/ac/olf parameters classes are a bit messy. Maybe we should try to clean that up in another PR as you already suggested. By putting apart the load part and using the OpenLoadFlowParameters for it?

Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@obrix
Copy link
Member Author

obrix commented May 21, 2021

@flo-dup I aggree, there is a a lot of unnecessary duplications in the parameters class, we should discuss it soon.

Signed-off-by: Bertrand Rix <bertrand.rix@artelys.com>
@@ -43,7 +43,7 @@ public String getElementType() {
@Override
public List<ParticipatingElement> getParticipatingElements(Collection<LfBus> buses) {
return buses.stream()
.filter(bus -> !(bus.isDisabled() || bus.isFictitious()))
.filter(bus -> !(bus.isDisabled() || bus.isFictitious() || !bus.isParticipating()))
Copy link
Member

Choose a reason for hiding this comment

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

The double negative is not well understandable...

Copy link
Member Author

@obrix obrix May 21, 2021

Choose a reason for hiding this comment

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

The alternative is bus.isParticipating() && !bus.isDisabled() && !bus.isFictitious(), I think it is better, do you agree ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes :-)

Signed-off-by: Bertrand Rix <bertrand.rix@artelys.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

93.3% 93.3% Coverage
0.0% 0.0% Duplication

@annetill annetill merged commit e5a43b4 into master May 21, 2021
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.

3 participants