Skip to content

Bugfix: matroid intersection approach to fix the order dependency of current greedy travesal approach#1450

Open
Jerry-Jinfeng-Guo wants to merge 14 commits into
mainfrom
bugfix/observability-check-redundant-if-condition
Open

Bugfix: matroid intersection approach to fix the order dependency of current greedy travesal approach#1450
Jerry-Jinfeng-Guo wants to merge 14 commits into
mainfrom
bugfix/observability-check-redundant-if-condition

Conversation

@Jerry-Jinfeng-Guo

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo commented Jun 24, 2026

Copy link
Copy Markdown
Member

The order of the neighbourhood information has decisive impact on the execution of the greedy graph traversal approach that is behind the current meshed observability check. This can be a source of error and yield false negatives when node degree is high and the order of the neighbourhood information has a impact on the tree constructed. This PR fixes that.

This is an experiment.

Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo self-assigned this Jun 24, 2026
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added the bug Something isn't working label Jun 24, 2026
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
…endence of the existing meshed check (graph walk)

Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo changed the title Bugfix: removed the redundant if condition and always add empty nodes Bugfix: DisjointSet approach to fix the order dependency of current greedy travesal approach Jun 29, 2026
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added the do-not-merge This should not be merged label Jun 29, 2026
@Jerry-Jinfeng-Guo
Jerry-Jinfeng-Guo marked this pull request as ready for review June 29, 2026 20:03
@Jerry-Jinfeng-Guo

Copy link
Copy Markdown
Member Author

Added do-not-merge. Pending on team.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses order-dependence in the meshed-network observability sufficient-condition check by introducing an order-independent approach based on contracting branch-measured edges (union-find) and applying a matroid-intersection style selection for assignable spanning edges. It adds targeted unit tests and introduces a new meshed-network observability test dataset.

Changes:

  • Add an order-independent meshed observability check (meshed_observable_matroid_intersection) and wire it into observability_check() for meshed networks.
  • Update starting-node candidate selection logic used by the legacy greedy traversal.
  • Add C++ unit tests for contraction, contracted-network construction, and matroid-intersection observability, plus new JSON test vectors (+ SPDX license sidecars).

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
power_grid_model_c/power_grid_model/include/power_grid_model/math_solver/observability.hpp Implements union-find contraction + matroid-intersection-based meshed observability; updates meshed path in observability_check; tweaks starting-node selection.
tests/cpp_unit_tests/math_solver/test_observability.cpp Adds unit tests covering the new contraction/contracted-network logic and order-independence scenarios.
tests/data/state_estimation/meshed-network-observability/08-starting-candidate-dominance/input.json Adds a meshed-network input dataset intended to reproduce order-sensitivity scenarios.
tests/data/state_estimation/meshed-network-observability/08-starting-candidate-dominance/input.json.license Adds SPDX license sidecar for the new input dataset.
tests/data/state_estimation/meshed-network-observability/08-starting-candidate-dominance/params.json Adds parameters for the new dataset.
tests/data/state_estimation/meshed-network-observability/08-starting-candidate-dominance/params.json.license Adds SPDX license sidecar for the new params dataset.
tests/data/state_estimation/meshed-network-observability/08-starting-candidate-dominance/sym_output.json Adds an (empty) expected output placeholder for the dataset.
tests/data/state_estimation/meshed-network-observability/08-starting-candidate-dominance/sym_output.json.license Adds SPDX license sidecar for the new output file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mgovers

mgovers commented Jun 30, 2026

Copy link
Copy Markdown
Member

AFAICT this grid is not observable

image

can you explain how it is observable? in addition, please reduce it to a minimal reproducible case.

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo changed the title Bugfix: DisjointSet approach to fix the order dependency of current greedy travesal approach Bugfix: matroid intersection approach to fix the order dependency of current greedy travesal approach Jul 13, 2026
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working do-not-merge This should not be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants