Skip to content

Complete PowerFactory contingency recording and screening - #95

Open
aswinkrishnapoyil wants to merge 1 commit into
Power-Agent:mainfrom
aswinkrishnapoyil:feat/powerfactory-contingency-recording-screening
Open

aswinkrishnapoyil wants to merge 1 commit into
Power-Agent:mainfrom
aswinkrishnapoyil:feat/powerfactory-contingency-recording-screening

Conversation

@aswinkrishnapoyil

@aswinkrishnapoyil aswinkrishnapoyil commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Context

This PR follows up on #89 and addresses the two optional items identified during its review:

  1. add_contingency_result_variables had no inverse operation.
  2. Non-string variable entries could be silently normalized or filtered instead of being rejected.

It also adds persistent configuration of PowerFactory contingency-screening criteria.

Changes

Complete the result-recording lifecycle

  • Add remove_contingency_result_variables.
  • Make removal idempotent.
  • Report removed and already-absent variables separately.
  • Reject non-string and blank variable entries, including their input indexes.
  • Preserve raw MCP input types so numeric values are not coerced into strings before validation.
  • Read configured variables from the underlying PowerFactory IntMon selections.
  • Retain ElmRes column inspection as a compatibility fallback.
  • Document both add and remove operations.

Handle stale ElmRes columns

Live PowerFactory testing exposed an important edge case:

  1. A variable is removed from its IntMon recording selection.
  2. Its old ElmRes column remains visible until the contingency analysis is rerun.
  3. The old add implementation sees that stale column and incorrectly reports that the variable is still recorded.

The add tool now treats the IntMon selections as the source of truth. This allows a removed variable to be added back immediately without running a calculation first.

Configure contingency screening

Add configure_contingency_screening, which can persist selected ComSimoutage screening settings without executing the analysis:

  • DC or AC-linearised screening method.
  • Simple loading criterion.
  • Simple loading threshold.
  • Combined loading criterion.
  • Combined loading threshold.
  • Relative loading-change threshold.
  • Ignore components overloaded in the base case.
  • Restrict screening to recorded elements.

The tool:

  • requires at least one requested change;
  • validates threshold values;
  • reads back every written value;
  • reports the previous and resulting settings;
  • reports whether anything changed;
  • rolls back the requested settings if a write or verification fails;
  • does not execute contingency analysis.

get_contingency_configuration now also returns these screening settings.

Documentation

Updated:

  • PowerFactory/README.md
  • PowerFactory/functions_overview.txt

Automated validation

PowerFactory suite

47 passed, 9 subtests passed

@qian-harvard qian-harvard 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.

Thanks. The remove tool and the screening configuration fill real gaps, and treating the IntMon selections as the source of truth to get past stale ElmRes columns makes sense.

I reviewed this against the fakes, since I don't have PowerFactory here. The inline questions about PowerFactory behaviour need your live setup.

Before merging, please address:

  1. A failed removal is reported as already_absent (remove_contingency_result_variables).
  2. One unreadable IntMon fails the whole add call, and remove reports it once per object.
  3. Whether class-level IntMon selections can exist. If they can, both tools miss them.

The rest is smaller: the list[Any] schema change, missing attributes reported as False, test coverage of the IntMon path, duplication between add and remove, and the configure tool's description.

"variable": variable,
"code": code,
})
(removed if removed_here else already_absent).append(variable)

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.

A failed removal ends up in already_absent. If RemoveVar raises, or returns something other than 0/None/1, for every matching monitor, removed_here stays False and the variable is filed as already absent.

With one monitor whose RemoveVar raises, the result is already_absent_variables: 1 and the message "Some result variables could not be removed; all requested variables were already absent; no change", while the variable is still recorded.

Suggest a third per-variable outcome (e.g. failed). Count a variable as absent only when every matching monitor returned 1, or no monitor matched.

recorded = {}
for monitor in monitors:
monitored_object = monitor.GetAttribute("obj_id")
if monitored_object is None:

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.

Question, since I can't check this without PowerFactory: can an IntMon in the result file select variables for a whole class (empty obj_id, class name set)?

If it can, both tools skip it here. Add would report a class-recorded m:u as added and ask for a rerun. Remove would report it as already_absent while it is still recorded. The FindColumn check this replaces would have seen those columns. If class-level selections are possible, they should count as recording the variable for every object of that class, or at least be reported.


recorded = {}
for monitor in monitors:
monitored_object = monitor.GetAttribute("obj_id")

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.

An exception from any one monitor (GetAttribute("obj_id"), NVars, GetVar, GetFullName) escapes this helper. It fails the whole add call as "PowerFactory read failed" before anything is configured, which undoes the per-object error scoping from #89.

In remove_contingency_result_variables, the same broken monitor is reported once per queried object instead: 3 identical errors for a 3-bus query, and up to 1000 with max_objects=1000. Suggest catching per monitor here, collecting those errors once, and having both tools report them the same way.

def add_contingency_result_variables(
object_query: str,
variables: list[str],
variables: list[Any],

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.

The PR description says list[Any] keeps numbers from being coerced to strings, but pydantic v2 doesn't do that coercion. With mcp 2.3.0 and list[str], ["m:u", 7, ""] is already rejected with variables.1 Input should be a valid string.

What list[Any] does change is the tool schema: the items go from {"type": "string"} to {}, so models lose the type hint. I'd keep list[str] here and in remove_contingency_result_variables. _contingency_variable_names is still worth keeping for blank and whitespace-only entries, which pydantic accepts.

"screening_method": {0: "dc", 1: "ac_linearised"}.get(
method, "unknown" if method is None else f"unknown:{method}"
),
"simple_loading_criterion": bool(_attribute(command, "scrCritSimple", 0)),

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.

On a PowerFactory build without these attributes, _attribute returns the default 0 and bool() turns it into False. So get_contingency_configuration reports the four criteria as disabled when they don't exist, while the thresholds correctly come back None.

Suggest returning None for missing booleans too, e.g. a small helper that returns None when the attribute is absent and bool(value) otherwise.



@mcp.tool()
def remove_contingency_result_variables(

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.

Most of this function's first ~50 lines duplicate add_contingency_result_variables: the method, variables and max_objects checks, then study case, ComSimoutage, result file and object query. It also re-implements the monitor-to-object matching instead of reusing an index like _contingency_recorded_variables.

The two already disagree. Add silently skips an object whose name can't be read, while remove reports it, and add has a fallback that remove doesn't. A shared helper for the preamble and one that maps full name to monitors would keep them aligned.

continue

matching_monitors = []
for monitor in monitors:

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.

This calls GetAttribute("obj_id") and GetFullName() for every monitor, once per object, so it makes O(objects × monitors) PowerFactory API calls on the single PF thread. With 1000 objects and ~1000 selections, that's about 2 million calls, and other tools wait behind them.

Building the full-name-to-monitors map once before the object loop, as add does, makes it linear.

ignore_base_case_overloads: bool | None = None,
screen_only_recorded_elements: bool | None = None,
) -> str:
"""Persist selected ComSimoutage screening settings without executing it."""

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.

This docstring is the tool description the model sees. Since this tool changes persistent study-case state, it needs more than one line. Please document each argument:

  • what dc vs ac_linearised screening means
  • that the thresholds are percentages
  • that None (or "" for the method) leaves a setting unchanged
  • that the change persists in the study case

The neighbouring contingency tools are a good model.

for monitor in monitors:
try:
monitored_object = monitor.GetAttribute("obj_id")
if monitored_object is obj or (

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.

Minor: unless the Python API reuses wrapper objects, monitored_object is obj only matches in the test fakes, and in PowerFactory the full-name comparison always decides. Comparing by full name alone, with a test that uses distinct-but-equal objects, would exercise the real path.

"query": query,
"variables": variable_names,
"total_objects": len(objects),
"configured_objects": len(configured),

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.

configured gets every queried object, including ones with no matching IntMon, so configured_objects is the number of objects examined. A query over 50 buses where 2 have selections reports 50, with 48 counted as already_absent.

In add, the same field counts only objects that were touched. Suggest counting only objects with a matching selection here, or reporting objects_without_selection separately.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants