Skip to content

Add tap regulator control side topology validation to Python validator#1293

Draft
Copilot wants to merge 11 commits intomainfrom
copilot/add-tap-regulator-validation
Draft

Add tap regulator control side topology validation to Python validator#1293
Copilot wants to merge 11 commits intomainfrom
copilot/add-tap-regulator-validation

Conversation

Copy link

Copilot AI commented Jan 29, 2026

Implementation Plan for Tap Regulator Control Side Validation

  • Study C++ tap_position_optimizer graph building logic
  • Add InvalidTapRegulatorControlSideError to validation errors
  • Implement transformer topology graph building in Python
  • Add validation function for tap regulator control side
  • Integrate validation into validate_transformer_tap_regulator
  • Add comprehensive tests for the new validation
  • Fix test cases to match correct logic
  • Move heapq import to top of file
  • Run security checks (no vulnerabilities found)
  • Fix mypy type inference error
  • Fix test expectation and lint errors
  • Refactor complex function to reduce complexity
  • Fix remaining mypy errors

Recent Changes

Fixed mypy errors:

  • Removed duplicate code that was accidentally left after refactoring (lines 1668-1749)
  • Added explicit type annotations for invalid_regulator_ids: list[int] in helper functions
  • Removed unused noqa directive

All mypy and Ruff checks now pass.

Original prompt

This section details on the original issue you should resolve

<issue_title>[FEATURE] Add tap regulator control side validation to data validator</issue_title>
<issue_description>## Summary

Add validation to the Python data validator to detect invalid tap regulator control side configuration
before running power flow calculations. This validation should catch cases where a transformer is being
controlled from the non-source side towards the source side, which is currently only detected during
runtime calculation.

Related Issues and PRs

Problem Statement

Current Behavior

Currently, when a tap regulator has an invalid control side configuration (controlling from non-source
side towards source side), the error is only detected during power flow calculation in the C++ core:

AutomaticTapInputError: Automatic tap changer has invalid configuration. 
The transformer is being controlled from non source side towards source side.
Transformer IDs: [123, 456, 789]

Why This Is a Problem

  1. Late error detection: Errors are only discovered after the model is constructed and calculation
    begins, wasting computation time

  2. Poor development experience: Developers must wait for calculation to start before discovering
    data issues

  3. Incomplete validation workflow: The Python data validator doesn't catch this error, creating a
    gap in pre-calculation validation

  4. Inconsistent with best practices: The documentation recommends validating data before
    construction, but this specific check is missing

User Impact

For users with large models (e.g., 50+ transformers with tap regulators):

  • Cannot validate all input data (wrt this regulated side) before construction
  • Must rely on trial-and-error with calculations to find configuration issues
  • Difficult to integrate into automated data validation pipelines

Proposed Solution

Add a new validation check in the Python data validator
(src/power_grid_model/validation/_validation.py) to detect invalid tap regulator control side
configurations.

Validation Logic

The validation should check for each transformer_tap_regulator in topology:

  1. Identify the regulated transformer (using regulated_object field)

  2. Determine the trafo-topology:

    • Simplified trafo graph: build with shortest distance in mind
  3. Validate control side:

    • The control_side must point to a side that is closer to or at the source
    • If controlling from a side that is farther from the source, append trafo user side ID
    • raise a validation error containing these trafo user side IDs

Backward Compatibility

  • New validation should be additive only
  • Existing validation should continue to work
  • Consider adding a warning first, then making it an error in a future release

</issue_description>

<agent_instructions>This issue will require a new topo graph building with reference to the c++ side transformer graph in tap_position_optimizer. Make sure to understand that front first. Create a branch for this: copilot-playground/add-transformer-regulated-side-check-in-python-validator</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@Jerry-Jinfeng-Guo I will experiment with Copilot in the issue to see if she's up to the task.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 4 commits January 29, 2026 14:10
Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Copilot AI changed the title [WIP] Add tap regulator control side validation to data validator Add tap regulator control side topology validation to Python validator Jan 29, 2026
@Jerry-Jinfeng-Guo
Copy link
Member

@copilot CI is still failing. Mypy is also reporting errors

Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jan 30, 2026

@copilot CI is still failing. Mypy is also reporting errors

Fixed the mypy error by adding explicit type annotation for the priority queue in Dijkstra's algorithm. The error was on line 1440 where mypy couldn't infer the type parameter for heappush.

Changes in commit 853a471:

  • Added type annotation: pq: list[tuple[float, int]] to explicitly specify the heap contains tuples of (distance, node_index)

@Jerry-Jinfeng-Guo
Copy link
Member

@copilot Pytest is still failing. Please fix.

Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Copy link
Author

Copilot AI commented Feb 1, 2026

@copilot Pytest is still failing. Please fix.

Fixed pytest and lint errors in commit 29dac1e:

Test fix: Corrected test_multiple_invalid_regulators expectation. The test was expecting both regulators 31 and 32 to be invalid, but regulator 32 is in a disconnected subgraph where both endpoints (nodes 2 and 3) are unreachable from the source. The validation correctly only flags regulator 31, which has one reachable endpoint (node 1) and one unreachable endpoint (node 2).

Lint fixes:

  • Removed unused imports: numpy and pytest
  • Fixed line length violations (> 120 chars) by moving inline comments to separate lines

@Jerry-Jinfeng-Guo
Copy link
Member

@copilot please address the complaints from Ruff-check. This includes a complexity refactoring of a long funciton.

…lexity issues

Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Copy link
Author

Copilot AI commented Feb 3, 2026

@copilot please address the complaints from Ruff-check. This includes a complexity refactoring of a long funciton.

Addressed all Ruff complaints in commit d9d37e7:

Complexity refactoring: Split the 300+ line validate_tap_regulator_control_side_topology function into 7 smaller, focused helper functions:

  • _build_regulated_transformer_mappings() - Build regulator mappings
  • _add_transformer_edges_to_graph() - Add 2-winding transformer edges
  • _add_three_winding_transformer_edges_to_graph() - Add 3-winding transformer edges
  • _add_branch_edges_to_graph() - Add line/link edges
  • _compute_distances_from_sources() - Run Dijkstra's algorithm
  • _check_two_winding_transformer_validity() - Validate 2-winding transformers
  • _check_three_winding_transformer_validity() - Validate 3-winding transformers

Fixed issues:

  • Reduced from 153 statements to 40 statements in main function
  • Fixed RUF015: Changed list slice to next(...)
  • Fixed SIM102: Combined nested if statements
  • Fixed RUF005: Used unpacking instead of concatenation

All Ruff checks now pass.

@Jerry-Jinfeng-Guo
Copy link
Member

Please fix the mypy errors @copilot

Co-authored-by: Jerry-Jinfeng-Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
Copy link
Author

Copilot AI commented Feb 3, 2026

Please fix the mypy errors @copilot

Fixed all mypy errors in commit ada84af:

Issues resolved:

  • Removed duplicate code (82 lines) that was accidentally left after refactoring
  • Added explicit type annotations: invalid_regulator_ids: list[int] in both _check_two_winding_transformer_validity() and _check_three_winding_transformer_validity() functions
  • Removed unused noqa directive

All mypy checks now pass with "Success: no issues found".

Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 3, 2026

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

Labels

do-not-merge This should not be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Add tap regulator control side validation to data validator

2 participants