Restore the resource-constraint check on interior periods

[vahid-ahmadi]

Why

_build_specs sets RC_TPI = 0.2 so TPI can finish despite a boundary artifact at the final period. But OG-Core compares RC_TPI against every period at once (np.any), so that also waives violations across the whole path — including magnitudes of the same order as the terminal error it was written for. A transition path that goes wrong mid-path returns numbers instead of raising.

RC_TPI also has a paramtools validator of range: [1e-13, 0.01]. 0.2 is 20x the schema maximum and only takes effect because it is assigned directly after update_specifications. There is no in-contract value that accommodates the terminal artifact, which is why this is handled after the solve rather than by tuning the parameter.

What changes after merging

A resource-constraint violation on the interior of the path, or a non-finite one anywhere, raises instead of passing silently — naming the magnitude and the period. Real runs are unaffected: verified directly against four solved production paths. RC_TPI = 0.2 is unchanged.

The measured error profile

Tolerances are set from data, not chosen. Measured across four production runs (S=80, J=7, T=60; baseline and a CIT reform, each under both TPI_outer_method settings):

t = 0       6.7e-03      initial conditions are imposed, not solved
t = 1       ~1e-07
t = 2       6.3e-04      largest genuine interior value
t >= 3      <= 3e-06
t = T-1     1.58e-01     truncation artifact

The terminal cause is now diagnosed: I_d[T-1] is formed from a steady-state-filled K_d[T] against the actual b_sp1[T-1], so the entire gap lands in that one period. The gap term accounts for it to better than 1 part in 3,800 across all four runs, and it shrinks as T grows relative to S (at T=160/S=40 it is 4.4e-06). Filed upstream as PSLmodels/OG-Core#1216.

This corrects the premise in #72 and in _build_specs' own comment, both of which claim "all other periods are well within 1e-4". They are not — t=0 and t=2 exceed it.

Change

_check_interior_resource_constraint, called after both the baseline and reform TPI solves:

• Non-finite errors always raise. nan >= tol is False, so without an explicit check a diverged path falls through every comparison. OG-Core's guards share this blindness — np.any(|RC_error| >= RC_TPI) and |TPIdist| > mindist_TPI are both False under NaN — so a diverged path is precisely what this must catch.
• t = 0 is checked against INITIAL_RC_TOL = 1e-2, not skipped. It is the impact year, so a genuine period-0 calibration inconsistency should not be invisible. It is exempt only when it is also the terminal period (a single-period path).
• Interior t in [1, T-2] against INTERIOR_RC_TOL = 5e-3 — about 8x headroom over the largest measured genuine value, while still catching both the terminal magnitude (0.16) and the 0.109 that motivated RC_TPI = 0.2.
• t = T-1 is exempt as a truncation artifact.

resource_constraint_error is (T, M), so trailing axes are collapsed and the reported index is a period.

Evidence

oguk/tests/test_resource_constraint.py — 19 tests, 0.15s. Includes a regression test built from the measured profile above (which fails against an earlier 1e-4-over-rc[:-1] version of this check), non-finite cases parametrised over ±nan and ±inf including in the exempt terminal period, a real violation at t=0 still raising, t=1 not being treated as exempt, trailing-axis collapse, and the degenerate inputs.

The check is also run directly against all four production TPI_vars.pkl outputs — all pass.

Full suite: 32 passed, 3 skipped. ruff format --check and ruff check clean.

Not verified

No test exercises run_transition_path end to end, so the two call sites are covered by reading rather than execution. Both sit after pickle.load and before _tpi_dict_to_result.

Fixes #72. If PSLmodels/OG-Core#1210 lands a separate terminal tolerance upstream, this can be simplified to set it and restore a tight RC_TPI.

[jdebacker]

@vahid-ahmadi This looks like a nice addition to the API. Will merge when you resolve conflicts with main.

[vahid-ahmadi]

@jdebacker Rebased on main — conflicts resolved. The clash was with #78 (now merged): both added a function at the same point in oguk/api.py, so both are kept, with tpi_outer_method first and the resource-constraint helper after it. No logic changed on either side.

Verified after the rebase: 13 tests pass across test_resource_constraint.py (9) and test_outer_method.py (4, from #78), and ruff format --check / ruff check are clean. Ready to merge.

[vahid-ahmadi]

@jdebacker Please don't merge the version you approved — I found a bug in it and have pushed a fix. Flagging rather than quietly amending, since your approval is against the earlier commit.

The problem. My original check used a 1e-4 tolerance over RC_error[:-1]. I only ever tested it against synthetic profiles. Measuring the real error profile from four production runs (S=80, J=7, T=60; baseline and a CIT reform, each under both TPI_outer_method settings):

t = 0       6.7e-03      initial-condition artifact
t = 1       ~1e-07
t = 2       6.3e-04      largest genuine interior value
t >= 3      <= 3e-06
t = T-1     1.58e-01     truncation artifact

rc[0] is 6.7e-03 — 67x my threshold. The approved version would have raised RuntimeError on every real transition path. It would have turned a silent-pass problem into a total failure, which is worse than what it set out to fix.

This also corrects the premise in #72 and in OG-UK's own in-code comment, both of which claim "all other periods are well within 1e-4". That is not true — t=0 and t=2 are not.

The fix. Exempt one period at each end, not just the terminal one, and set the interior tolerance from the measurement:

• t = T-1 is truncated. The mechanism is now diagnosed: I_d[T-1] is formed from a steady-state-filled K_d[T] against the actual b_sp1[T-1], so the whole gap lands in that one period. The gap term accounts for the terminal error to better than 1 part in 3,800 across all four runs. Filed upstream as Resource constraint is violated by ~0.16 at the terminal period of a transition path OG-Core#1216, with evidence that it shrinks as T grows relative to S (at T=160, S=40 the terminal error is 4.4e-06).
• t = 0 has its initial conditions imposed rather than solved.
• Interior tolerance 1e-3, which clears the genuine interior maximum of 6.3e-4 with margin while still catching a violation of either the terminal magnitude (0.16) or the interior magnitude RC_TPI = 0.2 hides.

Verification I should have done the first time: the fixed check is run directly against all four production TPI_vars.pkl outputs and passes on all of them. There is also a regression test built from the measured profile, which fails against the previous version.

12 tests, ruff clean, and the diff is still just oguk/api.py and the test file.

[vahid-ahmadi]

Pushed three more fixes from an independent review round, and updated the description (which had drifted from the code).

The one that matters: non-finite resource-constraint errors passed silently. nan >= tol is False, so a diverged solve fell through every comparison and returned a result with no exception — the worst class of bad path was the one this check waved through. OG-Core's own guards share the blindness, since np.any(|RC_error| >= RC_TPI) and |TPIdist| > mindist_TPI are also False under NaN. Now checked explicitly, including when the non-finite value sits in the exempt terminal period.

Two others: t = 0 is now checked against a looser 1e-2 rather than blanket-exempt — it is the impact year and a genuine period-0 inconsistency should not be invisible — and the interior tolerance is 5e-3 rather than 1e-3, since 1e-3 was only 1.58x above the largest measured value with that headroom set by a single calibration.

19 tests, all four production paths still pass, 32 passed across the suite.

[vahid-ahmadi]

The failing Test check is not from this PR — CI appears to be broken repo-wide.

DatasetMaterializationError: Could not download UK dataset 'enhanced_frs_2024_25':
Hugging Face rejected the configured credentials. Set HUGGING_FACE_TOKEN to a
token with access to the repository.

It fails in oguk/tests/test_get_micro_data.py, which this PR does not touch, and identically on #81 and #82. The last CI run on main (the #77 merge) passed, so the credentials look to have stopped working since then — an expired or rotated HUGGING_FACE_TOKEN secret rather than a code change.

Note the failure mode differs by environment: with no token at all those tests skip cleanly (that fix went in with #69), but with a token present and rejected they error. So CI is hitting the rejected-credential path.

Everything else is green on all three PRs, and the 32 passing tests here include all 19 for this change. Flagging because it will block merging anything until the secret is refreshed — happy to open a separate issue if that is more useful than a comment here.

[vahid-ahmadi]

@jdebacker The conflicts are resolved — this has been MERGEABLE/CLEAN since 31 August, rebased on main after #78 merged. Ready whenever you are.

Two things worth knowing before you merge, since both landed after your approval:

The check had a real bug, now fixed. The version you approved used a 1e-4 tolerance over RC_error[:-1], which I had only tested against synthetic profiles. Measuring the actual profile from four production runs showed rc[0] is 6.7e-03 — so it would have raised on every real transition path. It now exempts the terminal period, checks t=0 against a looser 1e-2 (it is the impact year, so skipping it entirely would hide a genuine period-0 inconsistency), and uses 5e-3 on the interior. Also catches non-finite errors explicitly: nan >= tol is False, so a diverged path was previously falling through every comparison.

The failing Test check is not from this PR. CI has been red repo-wide since around 31 August:

DatasetMaterializationError: Could not download UK dataset 'enhanced_frs_2024_25':
Hugging Face rejected the configured credentials. Set HUGGING_FACE_TOKEN to a token
with access to the repository.

It fails in oguk/tests/test_get_micro_data.py, which this PR does not touch, and identically on #81 and #82. The last green CI run on main was the #77 merge. With no token those tests skip cleanly (that went in with #69); with a token present and rejected they error — so the secret looks to have expired or been rotated rather than removed. Worth refreshing, as it currently blocks every PR in the repo.

Verified here: 19 tests for this change, the check run directly against all four production TPI_vars.pkl outputs (all pass), and 32 passing across the suite locally.