Skip to content

Changed SubgridContainer to represent galvanically seperated grids #1366

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

Conversation

staudtMarius
Copy link
Member

Resolves #1226
Resolves #1310

@staudtMarius staudtMarius self-assigned this Jul 9, 2025
@staudtMarius staudtMarius added bug Something isn't working enhancement New feature or request labels Jul 9, 2025
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

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

Almost there :)

Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

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

LGTM

@danielfeismann danielfeismann merged commit 2909188 into dev Jul 11, 2025
4 checks passed
@danielfeismann danielfeismann deleted the ms/#1226-change-SubgridContainer-to-be-in-line-with-the-galvanical-seperation branch July 11, 2025 14:46
@danielfeismann danielfeismann added this to the Version 8.0 milestone Jul 17, 2025
@danielfeismann
Copy link
Member

One thing we missed here is clarifying whether we consider the transformer to be part of the 'SubGridContainer', and if so, whether it is part of the lower or upper grid.

Excluding the transformer would make voltages within the container very simple and clear. However, this would mean that not all grid elements would be within the 'SubGridContainer'. I therefore tend to think that the connecting transformer should belong to the grid with the lower voltage level (but not the connecting node).

What do you think @sebastian-peter and @staudtMarius?

Once we agreed on a way we should update PSDM Docs and adapt OsmoGrid, which revealed this.

@sebastian-peter
Copy link
Member

@danielfeismann I think traditionally, we have (almost) handled it in the exact way you described. The transformer is supposed to be part of the lower sub grid. The upper connecting node of the transformer (which I think you meant there) is part of the sub grid as well, though, because otherwise we would have a connecting element (the transformer) for which one node (the upper) does not exist within the container.

Check out ContainerUtils#filterForSubnet for reference (filtering for transformer with lower node, but also adding the upper node):

Set<Transformer2WInput> transformer2w =
input.getTransformer2Ws().stream()
.filter(transformer -> transformer.getNodeB().getSubnet() == subnet)
.collect(Collectors.toSet());
/* Add the higher voltage node to the set of nodes */
nodes.addAll(
transformer2w.stream().map(Transformer2WInput::getNodeA).collect(Collectors.toSet()));

In my opinion, this approach is sufficiently well reasoned and should stay this way, however, there should definitely be documentation on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misconfigured nodes lead to "loops not allowed" Fix issue when creating JointGridContainer with SwitchInput
3 participants