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] Less internal connections created at import #3210

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

flo-dup
Copy link
Contributor

@flo-dup flo-dup commented Nov 13, 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?
Each connectable and switch is created with two nodes at each end, one for the cim:ConnectivityNode, one for the cim:Terminal. The two nodes are connected with an InternalConnection. This leads to a verbose iidm file with an excessive number of InternalConnections.

What is the new behavior (if this is a feature change)?
The InternalConnection is created only if needed:

  • if there's only one Connectable at the ConnectivityNode, no InternalConnection
  • if there are two Connectables at the ConnectivityNode, only one InternalConnection is created, for the first Connectable encountered (instead of one for each)
  • if there are more than two Connectables, one InternalConnection is created between each terminal and the ConnectivityNode, except for the BusbarSection case:
    • if there's exactly one BusbarSection among the connectables, the InternalConnection are created between this BusbarSection and the other connectables
    • if there's more than one BusbarSection (does not make so much sense, but exists in real data), the InternalConnections are created between the first BusbarSection and all the other Connectables, including the other BusbarSections.

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

  • Yes
  • No

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@flo-dup flo-dup requested a review from rcourtier November 13, 2024 14:43
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
@zamarrenolm
Copy link
Member

Just one quick consideration: could all these rules/simplification of the underlying connectivity model be moved to a post-processor after import (or a network modification). This way it could be useful for all importers, in a centralised way, and we should not need to consider them in every importer that have to handle. For example, recent versions of PSSE (35 and later) support node/breaker models where we could face similar problems.

@flo-dup flo-dup marked this pull request as ready for review February 19, 2025 15:37
@flo-dup flo-dup requested a review from olperr1 February 19, 2025 15:39
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
The node mapping is done at once for all terminals connected to a given connectivity node

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
71.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@flo-dup
Copy link
Contributor Author

flo-dup commented Feb 21, 2025

Just one quick consideration: could all these rules/simplification of the underlying connectivity model be moved to a post-processor after import (or a network modification). This way it could be useful for all importers, in a centralised way, and we should not need to consider them in every importer that have to handle. For example, recent versions of PSSE (35 and later) support node/breaker models where we could face similar problems.

Thanks for pointing out that similarity with PSSE! But we should rather centralize it in a utility method. Post-processing is not the good way to go I think (in terms of performance, robustness, and readability). That's at the cost of handling the utility method call (and map building beforehand) in each importer, and having an abstraction which allows it, sure. If that's not possible, I think we should rather duplicate the logic than putting it in a common postprocessing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for review
Development

Successfully merging this pull request may close these issues.

3 participants