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 double format for limit reduction values #2935

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

olperr1
Copy link
Member

@olperr1 olperr1 commented Mar 18, 2024

Please check if the PR fulfills these requirements

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

Limit reduction values are in float format in:

  • LimitViolation;
  • LimitReduction;
  • the methods taking a limit reduction value in LimitViolationUtils, the action DSL and in the API's equipment classes.

What is the new behavior (if this is a feature change)?
Limit reduction values are in double format.

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

  • 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)

  • LimitViolation.getLimitReduction now returns a double instead of a float;

  • If you have defined your own IIDM implementation, you should change the type of the limitReductionValue parameter from float to double in the checkPermanentLimit… and checkTemporaryLimit… methods of your Branch's and ThreeWindingTransformer's implementations;

  • If you have a custom LimitViolationDetector's implementation, you should change the type of the limitReductionValue parameter from float to double in the checkPermanentLimit and checkTemporaryLimit methods;

  • AbstractBranchActionExpressionNode.getLimitReduction() now returns a double instead of a float.

Other information:

The PR: do-not-merge tag was added since this PR relies on #2760. This latter should be merged prior to the current one.

Copy link
Member

@rolnico rolnico left a comment

Choose a reason for hiding this comment

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

Looks good !
New issues spotted by Sonar are unrelated, they probably come from the parent branch.

Could you maybe just add in the migration steps some notes for the methods from the action module that now take double parameters instead of float, since they are public ?

@olperr1
Copy link
Member Author

olperr1 commented Mar 18, 2024

Could you maybe just add in the migration steps some notes for the methods from the action module that now take double parameters instead of float, since they are public ?

float values can be used for double parameters when calling a method. Thus, the parameters' format change for the methods you are pointing out is not a breaking change.

Base automatically changed from limit_reduction to main March 18, 2024 14:02
Signed-off-by: Olivier Perrin <olivier.perrin@rte-france.com>
@olperr1 olperr1 force-pushed the double_format_for_limit_reduction branch from 2b7ae16 to 8e58e56 Compare March 18, 2024 14:18
Copy link

sonarcloud bot commented Mar 18, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
15.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@olperr1 olperr1 merged commit 1dc8644 into main Mar 18, 2024
5 of 6 checks passed
@olperr1 olperr1 deleted the double_format_for_limit_reduction branch March 18, 2024 14:38
rcourtier pushed a commit that referenced this pull request Mar 22, 2024
Signed-off-by: Olivier Perrin <olivier.perrin@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