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

CGMES: adjust multiple unpaired dangling lines connected at same boundary node #2737

Conversation

zamarrenolm
Copy link
Member

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?

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

When multiple unpaired dangling lines have the same boundary node (it means they are different alternatives from the same TSO), all of them receive the same value of (p0, q0), equal to the total equivalent injection defined at the boundary node.

What is the new behavior (if this is a feature change)?
After the import, the total equivalent injection is split between all dangling lines connected at the boundary node.
The adjustment is made after handling dangling lines only disconnected at boundary (that may be potentially disconnected also at network side if configuration indicates it).

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

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

.forEach(dls -> adjustMultipleUnpairedDanglingLinesAtSameBoundaryNode(dls, context));
}

private void adjustMultipleUnpairedDanglingLinesAtSameBoundaryNode(List<DanglingLine> dls, Context context) {
Copy link
Member

@annetill annetill Oct 4, 2023

Choose a reason for hiding this comment

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

After a talk with my colleague, it seems that it is not the good solution because the merge will not work. Imagine we have 3 DLs with the same pairing key (1 disconnected and 2 connected). On the other side, we have 2 DLs with the same pairing key (both connected). If they are disconnected, they are excluded from the merging. If they are connected, we do nothing without any error message. I think we have to:

  1. Warn the user during the merge in order to create by hand the remaining tie lines ;
  2. Or :
  • Put all the P0 and Q0 on one DL that keep the good pairing key ;
  • For the other ones connected, we put zero and we change the pairing key (with #0 or something else). But which one chosen ? And on the other side ? The merge will be done in creating a tie line of 2 arbitrary dangling lines (and maybe not the good ones).

I am lost :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

But the merge scenario you discuss has nothing to do with this change.
This change just improves on how do you see the ONLY equivalent injection that exists in one IGM, when you have MULTIPLE (all connected) dangling lines.
I would discuss separately the function required for perform a merge when multiple dangling lines from each side exist at a single boundary point.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the cleanest solution for the scenario you describe is that the user creates a modified boundary file, with a new connectivity/topological node. The user MUST then specify which lines go to each boundary node, in a way that always leaves only one line of each IGM at each boundary node.

If we do not want to modify the boundary file and the IGM to use the new (not "official" boundary point), then we can arbitrarily try to merge any pair of candidates or let the user configure it in some way. But this seems quite "dirty" for me: what happens if we have two connected lines in one side and only one in the other? How do we let the user specify the "correct" pairs to be merged? a parameter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Going a little bit further about merging multiple dangling lines in the same node:

We can try to pair the dangling lines from each side that have "closer values" of (p0, q0) (similar equivalent injection).
Following this idea, If there are 2 lines on one side and only one in the other: a tie line will be created, and a dangling line will be left on one side.

What do you think of this alternative ?
In any case, I think these considerations should go in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

We are going to open a new PR to increase merge logging, in order to warn the user that he has to do something!

zamarrenolm and others added 6 commits October 5, 2023 09:29
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
…oundary_node' of https://github.com/powsybl/powsybl-core into cgmes_adjust_multiple_unpaired_dangling_lines_at_same_boundary_node
Signed-off-by: Luma <zamarrenolm@aia.es>
…oundary_node' of https://github.com/powsybl/powsybl-core into cgmes_adjust_multiple_unpaired_dangling_lines_at_same_boundary_node
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 5, 2023

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

93.6% 93.6% Coverage
0.0% 0.0% Duplication

@annetill annetill merged commit 66f07f0 into main Oct 5, 2023
6 checks passed
@annetill annetill deleted the cgmes_adjust_multiple_unpaired_dangling_lines_at_same_boundary_node branch October 5, 2023 11:42
olperr1 pushed a commit that referenced this pull request Oct 6, 2023
…me boundary node (#2737)

Signed-off-by: Luma <zamarrenolm@aia.es>
(cherry picked from commit 66f07f0)
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