Act on power flow convergence instead of always reporting solved

[luke-kiernan]

Fixes #305 and #308. Depends on Sienna-Platform/InfrastructureOptimizationModels.jl#168.

#305 — convergence was thrown away. IOM.evaluate! ignored what PFS.solve_power_flow! reported and set is_solved = true unconditionally. It now reads PFS.get_converged(pf_data), sets is_solved = all(converged), and @errors with the failed time steps. Reading get_converged rather than the return value because the DC solve_power_flow! methods return nothing; both AC and DC populate converged. The single-period branch (PSSEExporter) has no convergence to report and still sets true.

A non-converged power flow does not fail the run. It is an error in the log and NaN in the aux variables: PowerFlows already blanks the diverged time steps (OVERWRITE_NON_CONVERGED), and the optimization itself solved fine, so solve! still returns SUCCESSFULLY_FINALIZED. This is also strictly more than the old code gave you, where one bad time step threw and erased all the aux var data. Per-step tolerance is the remaining open part of #273.

#308 — the read-back picked an arbitrary evaluator. latest_solved_power_flow_evaluation_data found one with findlast over a Dict, whose values order is arbitrary with more than one entry. Keys owned by the evaluator it didn't return hit the ownership filter's || return and were silently never written. IOM's calculate_aux_variables! now passes down the evaluator it is already holding, so that helper is deleted: the ownership filter moves onto a 4-arg method and the two concrete writers become _write_aux_variable_value!.

Tests. The non-convergence case builds a CopperPlate case whose LP is unaffected by the network, with two lines weakened enough that only the peak hours diverge — so all(converged) is doing real work, not just distinguishing all-pass from all-fail — and asserts NaN aux vars at exactly those hours. Tuning is noted in a comment; it is calibrated against the current NR solver. Two multi-evaluator testsets cover #308: AC + PSS/E export (the realistic pairing, and the worst case for findlast — the exporter owns no aux vars, so picking it skipped everything), and AC + DC, which checks order-independently that the DC evaluator leaves an AC-only key alone while the AC evaluator writes it.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The convergence handling and regression coverage address the stated behavior change with no blocking issues identified.

Pull request overview

Updates power-flow evaluation status to reflect actual multi-period convergence, preventing false successful solves.

Changes:

• Marks evaluations solved only when all time steps converge.
• Logs failed time steps while preserving single-period exporter behavior.
• Adds regression tests for full and partial convergence.

File summaries

File	Summary
test/test_power_flow_in_the_loop.jl	Tests successful and partial convergence behavior.
ext/PowerFlowsExt/pf_solve_and_aux.jl	Applies convergence status to power-flow evaluations.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[acostarelli]

Why say solve! failed when PF didn't converge but the PCM did? I feel like those should be separate: solve can still succeed but pf.is_solved may be false.

[jd-lara]

Why say solve! failed when PF didn't converge but the PCM did? I feel like those should be separate: solve can still succeed but pf.is_solved may be false.

PFs can fail for a multitude of reasons on a feasible decision model

[acostarelli]

For sure, but why are we saying that the whole solve! failed when only the post-solve PF failed? @jd-lara

[luke-kiernan]

For sure, but why are we saying that the whole solve! failed when only the post-solve PF failed?

Oh I missed that detail when opening this PR via Claude code. Yeah that's a pretty big change. This is really pending a design choice: see comment here and the alternative I laid out here...but based on the issue Jose opened, I was inferring that failure-to-solve should be treated as an error.

I guess the issue that latest_solved_power_flow_evaluation_data is now out of sync with what is_solved means. latest wants it to mean "there's good data from which we can calculate aux variables, i.e. most [any?] PFs converged." But now is_solved means all converged. Maybe I should just rework latest_..._data: the function has other issues, too (#308).

[jd-lara]

For sure, but why are we saying that the whole solve! failed when only the post-solve PF failed? @jd-lara

Because the PFs in PSI are aux variables, we don't want to stop a simulation that uses a DC assumption to fail because it can't recover all the steps of an AC PF for example.

[jd-lara]

but based on the issue Jose opened, I was inferring that failure-to-solve should be treated as an error -> yes but not a simulation fail. We talked about using NaN or something else in the aux vars

[github-actions]

Performance Results

Version	Precompile Time
Main	3.327997414
This Branch	3.225707334

Version	Build Time
Main-Build Time Precompile	85.382972222
Main-Build Time Postcompile	1.089360976
This Branch-Build Time Precompile	90.934515658
This Branch-Build Time Postcompile	1.112350778

Version	Solve Time
Main-Solve Time Precompile	125.716360283
Main-Solve Time Postcompile	91.815497877
This Branch-Solve Time Precompile	318.867894476
This Branch-Solve Time Postcompile	270.782097565

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[jd-lara]

@luke-kiernan we need to rebase this consider the Enum-X change