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

Flow decomposition observers refactoring #171

Merged
merged 8 commits into from
Oct 18, 2024
Merged

Flow decomposition observers refactoring #171

merged 8 commits into from
Oct 18, 2024

Conversation

caioluke
Copy link
Member

@caioluke caioluke commented Oct 15, 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)

What kind of change does this PR introduce?

Feature

What is the current behavior?

Currently, while running a flow decomposition, we can only retrieve intermediate results for branches and nodes. We can't retrieve an intermediate state for a boundary node, for example.

What is the new behavior (if this is a feature change)?
Flow decomposition observers now have full access to the network after an AC LF and a DC LF. Therefore, the user can retrieve any information they need from the network at those states.

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)

Implement the new methods from the FlowDecompositionObserver interface

  • computedAcLoadFlowResults
  • computedDcLoadFlowResults)

As well as remove the old ones

  • computedAcNodalInjections
  • computedDcNodalInjections
  • computedAcFlowsTerminal1
  • computedAcFlowsTerminal2
  • computedDcFlows
  • computedAcCurrentsTerminal1
  • computedAcCurrentsTerminal2)

Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke caioluke added the Breaking change API is broken label Oct 15, 2024
@caioluke caioluke requested a review from OpenSuze October 15, 2024 13:13
@caioluke caioluke self-assigned this Oct 15, 2024
caioluke and others added 4 commits October 15, 2024 15:15
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
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 this PR !

Signed-off-by: Caio Luke <caioluke97@gmail.com>

// Losses compensation
compensateLosses(network);

// DC load flow
runDcLoadFlow(network);
saveDcReferenceFlow(flowDecompositionResultsBuilder, network, xnecList);
saveDcLoadFlowResults(flowDecompositionResultsBuilder, network, xnecList, loadFlowServiceAcResult);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about using loadFlowServiceAcResult here ... ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ops, fixed it!

Comment on lines 86 to 88
if (observers.isEmpty()) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think some of those tests are not relevant anymore (line 86 and 96 for example)

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, well spotted

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you test the loadFlowResult ? At least that they are not null ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Copy link

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 this PR, I approve this !

@phiedw phiedw merged commit 8b00ffe into main Oct 18, 2024
7 checks passed
@phiedw phiedw deleted the refactor_observers branch October 18, 2024 12:53
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