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.

Sign up for GitHub

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

Jump to bottom

Add DecomposedFlow rescaler proportional to max AC current load #156

Merged
merged 18 commits into from
Sep 20, 2024
Merged

Add DecomposedFlow rescaler proportional to max AC current load #156

merged 18 commits into from
Sep 20, 2024

Conversation

caioluke
Copy link
Member

@caioluke caioluke commented Jul 18, 2024 •
edited
Loading

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)

What kind of change does this PR introduce?

Feature

What is the current behavior?

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

  • Refactor DecomposedFlowRescaler API
  • Add new DecomposedFlowRescaler based on the maximum current overload
  • Retrieve acFlows for both terminals in FlowDecompositionObserver
  • Retrieve acCurrents for both terminals in FlowDecompositionObserver

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

  • Yes
  • No

@caioluke
add new rescaler
c0afe30 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke caioluke requested a review from OpenSuze July 18, 2024 08:10
@caioluke caioluke self-assigned this Jul 18, 2024
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarCloud

@OpenSuze
Copy link
Contributor

OpenSuze commented Aug 26, 2024 •
edited
Loading

Hi @caioluke,
Sorry for the delay, I was in training and vacations.
Could you share some details about your metier needs that justify adding another rescaler ?
To me it seems that this new rescaler requires to make a bit too much modification on DecomposedFlow (to add a lot of new information).
Best,
Hugo

@caioluke
Copy link
Member Author

Welcome back, Hugo :)

Yes, indeed this PR adds quite some new stuff in the rescaler.

What I need is to rescale DC flows in such a way that I get the same level of current overload as in the AC case.

Therefore, I compare AC current overloads to find the terminal (1 or 2) with the highest current overload, and get the associated active power to rescale DC flows.
In case there are missing limits, I just compare AC currents.

Another subtlety is that I need to calculate these rescaling AC active powers with nominal voltage - as I have done in the code.

OpenSuze
OpenSuze requested changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@OpenSuze OpenSuze left a comment

Choose a reason for hiding this comment

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

Thank you for the PR !

Sorry for the delay, I had other projects to attend to...

Could you rename your PR with a less generic name ?

I know that I ask for a lot of modifications.

I do not what to do another PR to fix yours.

Feel free to ask more details about this feedback

flow-decomposition/src/main/java/com/powsybl/flow_decomposition/DecomposedFlow.java Outdated Show resolved Hide resolved
flow-decomposition/src/main/java/com/powsybl/flow_decomposition/DecomposedFlow.java Outdated Show resolved Hide resolved
flow-decomposition/src/main/java/com/powsybl/flow_decomposition/DecomposedFlow.java Outdated Show resolved Hide resolved
flow-decomposition/src/main/java/com/powsybl/flow_decomposition/DecomposedFlowBuilder.java Outdated Show resolved Hide resolved
flow-decomposition/src/main/java/com/powsybl/flow_decomposition/DecomposedFlowBuilder.java Outdated Show resolved Hide resolved
...n/java/com/powsybl/flow_decomposition/rescaler/DecomposedFlowRescalerMaxCurrentOverload.java Show resolved Hide resolved
...n/java/com/powsybl/flow_decomposition/rescaler/DecomposedFlowRescalerMaxCurrentOverload.java Show resolved Hide resolved
flow-decomposition/src/main/java/com/powsybl/flow_decomposition/FlowComputerUtils.java Outdated Show resolved Hide resolved
flow-decomposition/src/main/java/com/powsybl/flow_decomposition/FlowComputerUtils.java Outdated Show resolved Hide resolved
flow-decomposition/src/test/java/com/powsybl/flow_decomposition/TestUtils.java Outdated Show resolved Hide resolved
@caioluke caioluke changed the title Add new DecomposedFlow rescaler Add DecomposedFlow rescaler proportional to max AC current load Sep 6, 2024
caioluke and others added 10 commits September 9, 2024 15:45
@caioluke
add branch to DecomposedFlow
dbe88e3 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
changes after comments 1
08a9b70 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
refacto DecomposedFlowRescaler
940c69e 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
minor change
4365d71 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
remove Pair from FlowDecompositionObserver
7d2e1f7 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
add test, solve minor issues
78feec5 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
checkstyle fix
f60acf6 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
add test with fixed result
6c3a36f 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
remove minor issue
aefa666 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
Merge branch 'main' into max_current_overload_rescaler
a26a6f1
OpenSuze
OpenSuze requested changes Sep 12, 2024
View reviewed changes
Copy link
Contributor

@OpenSuze OpenSuze left a comment

Choose a reason for hiding this comment

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

Thank you for your changes ! I appreciate your new DecomposedFlowRescaler API :)

flow-decomposition/src/main/java/com/powsybl/flow_decomposition/FlowDecompositionObserver.java Outdated Show resolved Hide resolved
...sition/src/main/java/com/powsybl/flow_decomposition/rescaler/AbstractDecomposedRescaler.java Outdated Show resolved Hide resolved
...sition/src/main/java/com/powsybl/flow_decomposition/rescaler/AbstractDecomposedRescaler.java Outdated Show resolved Hide resolved
...main/java/com/powsybl/flow_decomposition/rescaler/DecomposedFlowRescalerAcerMethodology.java Outdated Show resolved Hide resolved
...n/java/com/powsybl/flow_decomposition/rescaler/DecomposedFlowRescalerMaxCurrentOverload.java Outdated Show resolved Hide resolved
...sition/src/main/java/com/powsybl/flow_decomposition/rescaler/DecomposedFlowRescalerNoOp.java Outdated Show resolved Hide resolved
...rc/main/java/com/powsybl/flow_decomposition/rescaler/DecomposedFlowRescalerProportional.java Outdated Show resolved Hide resolved
...rc/main/java/com/powsybl/flow_decomposition/rescaler/DecomposedFlowRescalerProportional.java Outdated Show resolved Hide resolved
flow-decomposition/src/test/java/com/powsybl/flow_decomposition/RescalingTests.java Show resolved Hide resolved
flow-decomposition/src/test/java/com/powsybl/flow_decomposition/TestUtils.java Outdated Show resolved Hide resolved
@OpenSuze
Copy link
Contributor

Could you add a bit of documentation please ?

@caioluke
dcFlows for observers, extract static methods for DecomposedFlowResca…
3a3c36a 
…lers

Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
add some documentation
8bd347c 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
Copy link
Member Author

caioluke commented Sep 13, 2024 •
edited
Loading

Could you add a bit of documentation please ?

@OpenSuze
I added some documentation.
I was wondering what to write about the parameter proportional-rescaler-min-flow-tolerance, since now it's also used by the new Rescaler...
Do you have a suggestion? Maybe change it's name?

@OpenSuze
Copy link
Contributor

Could you add a bit of documentation please ?

@OpenSuze I added some documentation. I was wondering what to write about the parameter proportional-rescaler-min-flow-tolerance, since now it's also used by the new Rescaler... Do you have a suggestion? Maybe change it's name?

Thank you for the documentation !

I think that I am ok with the parameter name. Both rescalers have some kind of proportionality.

@caioluke
minor changes
74f6a29 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
move min flow tolerance to flow-dec-parameters
cb48142 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke
remove created issue
61a4e1e 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
OpenSuze
OpenSuze approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@OpenSuze OpenSuze left a comment

Choose a reason for hiding this comment

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

Thank you for your PR !

@caioluke
add default rescale with null network (avoids breaking change)
384a255 
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@phiedw phiedw merged commit da53ae2 into main Sep 20, 2024
7 checks passed
@phiedw phiedw deleted the max_current_overload_rescaler branch September 20, 2024 07:08
@OpenSuze OpenSuze mentioned this pull request Sep 23, 2024
Closed
@caioluke caioluke mentioned this pull request Sep 25, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OpenSuze OpenSuze OpenSuze approved these changes

Assignees

@caioluke caioluke

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@caioluke @OpenSuze @phiedw