Skip to content

Fixed component graph validation to allow dangling components #1205

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

Closed
wants to merge 1 commit into from
Closed

Fixed component graph validation to allow dangling components #1205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,10 @@

## Summary

<!-- Here goes a general summary of what this release is about -->

## Upgrading

<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->

## New Features

<!-- Here goes the main new features and examples or instructions on how to use them -->

## Bug Fixes

<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
* Fixed component graph validation to allow dangling components. Previously, a strict tree (connected acyclic undirected graph) structure was required, but now components can exist without being connected to anything.
19 changes: 8 additions & 11 deletions src/frequenz/sdk/microgrid/component_graph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,7 @@ def _validate_graph(self) -> None:
InvalidGraphError: If:
- There are no components.
- There are no connections.
- The graph is not a tree.
- The graph is not a tree (but tree with dangling components is allowed).
- Any node lacks its associated component data.
"""
if self._graph.number_of_nodes() == 0:
Expand All @@ -899,7 +899,7 @@ def _validate_graph(self) -> None:
raise InvalidGraphError("No connections in component graph!")

if not nx.is_directed_acyclic_graph(self._graph):
raise InvalidGraphError("Component graph is not a tree!")
raise InvalidGraphError("Component graph has a cycle!")

# This check doesn't seem to have much sense, it only search for nodes without
# data associated with them. We leave it here for now, but we should consider
Expand All @@ -918,15 +918,12 @@ def _validate_graph(self) -> None:
if sum(1 for _ in self.connections()) <= 0:
raise InvalidGraphError("Graph must have a least one connection!")

Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a comment explaining why unconnected components now only trigger a warning rather than an exception, to clarify that dangling components are allowed.

Suggested change
# Allow dangling components (components with no connections) in the graph.
# These components are ignored during validation but logged as warnings
# to inform the user. This behavior is intentional to support use cases
# where some components might not be connected but are still part of the
# microgrid configuration.

Copilot uses AI. Check for mistakes.
# should be true as a consequence of the tree property:
# there should be no unconnected components
unconnected = filter(
lambda c: self._graph.degree(c.component_id) == 0, self.components()
)
if sum(1 for _ in unconnected) != 0:
raise InvalidGraphError(
"Every component must have at least one connection!"
)
for component in self.components():
if self._graph.degree(component.component_id) == 0:
_logger.warning(
"Component %s has no connections and will be ignored",
component,
)

def _validate_graph_root(self) -> None:
"""Check that there is exactly one node without predecessors, of valid type.
Expand Down
Loading