Stop the TPI outer loop early when it has stalled, with a diagnosis#1178
Open
marcelolafleur wants to merge 3 commits into
Open
Stop the TPI outer loop early when it has stalled, with a diagnosis#1178marcelolafleur wants to merge 3 commits into
marcelolafleur wants to merge 3 commits into
Conversation
Adds solvers.diagnose_stall, a window check on the existing TPIdist_vec history: when the best distance over the last TPI_stall_window iterations (default 50; 0 disables) has not improved on the best from before the window, the loop is diagnosed as stalled. The message distinguishes a cycling outer loop (suggest a lower nu or TPI_outer_method='anderson') from a diverging economy (usually an inconsistent fiscal block, which solver settings cannot fix). The default TPI_stall_action='warn' logs the diagnosis once and leaves model solutions unchanged; 'stop' also ends the loop early so the run fails through the existing ENFORCE_SOLUTION_CHECKS path instead of spending the rest of maxiter. Works identically for the picard and anderson update rules. Verified on the motivating case from issue PSLmodels#1177 (an OG-PHL multi-industry reform at nu=0.4): the stall is diagnosed at iteration 100 and the run stops, where the same configuration previously churned past iteration 203 of 250; the converging baseline transition (~76 iterations) triggers nothing.
marcelolafleur
marked this pull request as draft
July 21, 2026 16:20
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1178 +/- ##
==========================================
- Coverage 72.92% 72.76% -0.17%
==========================================
Files 22 22
Lines 5525 5551 +26
==========================================
+ Hits 4029 4039 +10
- Misses 1496 1512 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…ne growth Live testing showed the monotone test never fires on real models: a fiscal runaway either produces NaN within an iteration (already caught by the existing loop guards and RC check) or manifests as a widening bounce, which strict monotonicity misreads as cycling. The diverging diagnosis now fires when the recent window's best distance is far above the earlier best (default 2x) -- deterioration, not monotonicity, is the signature that the path is drifting away. A converging run with a temporary +8pp-of-GDP spending shock stays silent (no false positive), and a new unit test covers the widening-bounce case the old rule got wrong.
…t changes The final RuntimeError now appends the stall diagnosis and its cure (cycling: lower nu / anderson; diverging: check the fiscal block), so the guidance reaches users who only see the traceback. The warn-mode log fires once per diagnosis and again only if it changes (e.g. escalates from cycling to diverging), replacing the boolean once-per-stall guard with a smaller comparison against the last reported label.
marcelolafleur
marked this pull request as ready for review
July 21, 2026 20:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1177.
When the TPI outer loop stops making progress, nothing today says so until
maxiteris exhausted. This PR adds a window check on the existing distance history: if the best distance over the lastTPI_stall_windowiterations (default 50; 0 disables) hasn't improved on the best from before the window, the loop is diagnosed as stalled.The diagnosis distinguishes the two causes, because they have different cures: a recent best far above the earlier best means the path is drifting away — a diverging economy, usually an inconsistent fiscal block (spending, revenue, and
debt_ratio_ss), which solver settings cannot fix — while a bounce around the earlier best means the outer loop is cycling (try a lowernu, orTPI_outer_method="anderson"). The diagnosis is logged once, re-logged only if it changes, and carried into the finalRuntimeErrormessage so it reaches users who only see the traceback.The default action is
TPI_stall_action="warn": log and continue, leaving model solutions unchanged. Opting into"stop"ends the loop early, so a hopeless run fails through the existingENFORCE_SOLUTION_CHECKSpath instead of spending the rest ofmaxiter— no new failure semantics.The check is a pure function,
ogcore.solvers.diagnose_stall, and works identically for the picard and anderson update rules.Testing. Beyond the unit tests, the full local golden suite (
test_TPI.py -m local, 22 cases) passes unchanged under the warn default, and the feature was exercised live on OG-PHL's multi-industry model: a reform atnu=0.4that previously churned past iteration 203 of 250 is diagnosed at iteration 100 and (with"stop") ends there; a converging baseline and a stressed-but-recoverable run (+8pp of GDP spending shock) stay silent; and a growing limit cycle under picard atnu=0.45fires the warn-mode diagnosis exactly once while the run proceeds to its normal failure. Two live findings shaped the design: fiscal runaways violent enough to diverge produce non-finite values within an iteration (already caught by existing guards, so the stall detector correctly doesn't own that case), and real divergence shows up as a deteriorating distance band rather than monotone growth — so the diverging test is deterioration-based (recent best ≥ 2× earlier best). The diverging label is validated on synthetic histories; on live runs it is rare by nature, for the reasons above.cc @jdebacker @rickecon