BUG Aggregate the continuation lottery stably under a certainty equivalent - #413
Closed
hmgaudecker wants to merge 10 commits into
Closed
hmgaudecker wants to merge 10 commits into
hmgaudecker wants to merge 10 commits into
Conversation
Red tests for the pro-review's P0 findings before any production fix: process-only continuation targets silently dropped (KeyError on .solve()), sub-annual activity disagreement between the reachability graph and AgeGrid.get_periods_where, a coarse stochastic state law skipped when the regime has no retained regime-transition target that period, dual-violation error-message ordering, and a bare KeyError on an incomplete period_to_regime_to_V_arr during simulation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
…dge status Model.__init__ now evaluates every regime's active predicate exactly once via AgeGrid.get_periods_where and threads the resulting mapping through broadcast pruning, model-input validation, age normalization, and reachability-graph construction. reachability.py no longer evaluates Regime.active on exact-Fraction ages (active_periods_from_predicates removed) -- the sub-annual Fraction-vs-float32 disagreement between the graph and every other activity consumer is gone by construction, not by coincidental agreement. Also removes EdgeStatus.TRUE and build_phase_reachability's unconditional_targets_by_source: no current declaration proves unconditional positive probability independently of state/action/runtime parameters, and a one-key per-target mapping is not such a proof. Every retained edge is now CONDITIONAL. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
carry_targets derived a source's continuation targets from flat_nested_transitions -- keys present only when a target already has a non-process state-law bundle. A target reached solely by carrying a shared stochastic-process state (no other law) was silently omitted, so its intrinsic weight_<target>__next_<process> / <target>__next_<process> functions were never built; .solve() then crashed with KeyError: 'next_<state>' the moment it needed that continuation. _process_regime_core now takes the phase's PhaseReachability and the source regime name, and derives continuation targets from phase_reachability.union_targets(source=...) -- the graph, not law-bundle key presence. Existence of a continuation target is now a property of the reachability graph; an unrelated state's law-of-motion entry can no longer add or remove one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
_simulate_regime_in_period read a single active_regimes_next_period tuple (computed from the simulation graph) for two different jobs: assembling next_regime_to_V_arr for decision/Q evaluation, and constraining the realized regime draw. Per the model's own semantics, decisions must read solution-graph continuation values; only the realized draw belongs to the simulation graph. Splits the single tuple into decision_targets_next_period (regime.solution.reachability.targets) and realization_targets_next_period (regime.simulation.reachability.targets). Also replaces the bare dict-comprehension index into a caller-supplied period_to_regime_to_V_arr with _require_next_period_values, which raises a descriptive InvalidSimulationInputError naming the source regime, period, required solution-graph targets, and which ones are missing -- instead of a bare KeyError or a silent zero/empty substitution. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
Reorders _validate_regime_transition_probs back to main's range → sum → inactive-target check order (the branch had inactive-target running before sum-to-1, changing which error surfaces on a doubly-broken transition), makes _state_transition_unused_in_period check target-regime-none coarse laws regardless of an empty period-target set, replaces _classify_state_handoff's unread 4-way label with a plain bool (_has_valid_state_handoff), and formats handoff error messages with a decimal age instead of a raw Fraction. Isolates test_positive_granular_probability_outside_graph_is_rejected to the inactive-target check alone (probabilities now sum to exactly 1.0), since the restored check order means a doubly-broken transition reports the sum error first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
Records, in CHANGES.md and the Regime.transition / MarkovTransition docstrings, that a bare callable or bare MarkovTransition declares conservative support over every regime active in the next period, so every temporally compatible candidate needs a valid state handoff; a per-target dict is the only way to narrow that support, since runtime-zero probabilities don't. Explains why test_initial_conditions_process_grid_heterogeneous_state_sets uses a per-target dict rather than a bare transition. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
Path.open, read_text, and write_text without an explicit encoding decode as the locale default, which is cp1252 on Windows. A snapshot directory name carrying any non-ASCII character raised when writing REPRODUCE.md, and the metadata.json write/read pair spanning the internal writer and the public load_snapshot reader was never the locale's to choose either. Ports the fix already merged to feat/dcegm (3934701) onto reachability, since this branch diverged from main before that fix and has no PR of its own yet, so its public metadata.json writer would otherwise ship the same latent Windows failure if it reaches main ahead of feat/dcegm. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
…llback Drops the now-unneeded `# type: ignore[operator]` on the AgeSpecializedGrid union in user_regime_validation.py (ty accepts the plain union). Replaces backward_induction.py's `getattr(regime.solution, "period_state_axes", None)` duck-typing fallback with direct attribute access, giving MockSolutionPhase the real field so a missing one fails loudly instead of silently defaulting. Extracts the recursive tree-signature walk shared by age_specialization.py's tree_signature (age) and age_normalization.py's periodized_tree_signature (period) into one _tree_signature(tree, leaf_signature=...) helper, so both wrappers are thin callers of the same recursion instead of duplicating it. Extracts the continuation_info(period) and group_key(period) closures duplicated between processing.py's Q_and_F construction and diagnostics.py's NaN-path intermediates into continuation_info_lookup and continuation_group_key in age_normalization.py, so the primary solve and the diagnostic recomputation are guaranteed to group periods identically by construction rather than by two independently maintained closures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
Audits every remaining target-derivation call site (flat_nested_transitions, nested_transitions, carry_targets, active_regimes_next_period, get_periods_where/.active) and confirms each is either the single canonical activity-evaluation point, a legitimate graph query/consumer, or unrelated function-name/parameter-name partitioning — no semantic target set still originates from law-bundle keys or an independent activity re-evaluation. Pins that audit with four architecture-guard tests alongside the existing AST guards in test_reachability.py: solver runtime modules never call an activity predicate or inspect .transition/.state_transitions directly, the banned carry_targets identifier does not reappear, and get_periods_where + Regime.active are combined at exactly one call site project-wide. Documents the graph's construction-time, conditional-only, conservative- coarse-support, phase-independent, solver-consumes-but-never-infers contract in reachability.py's module docstring and in a new "Reachability" section of docs/explanations/architecture.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7rvG9fy53DdKW28jam2zL
…alent `Q_and_F` built the continuation as `inverse(Σ w · transform(v))` — per-target transform, weighted reduction, one final inverse. Under `PowerMean` with `risk_aversion > 1`, the intermediate `v^(1-ra)` overflows the dtype before any summation happens whenever continuation values are small, so a finite certainty equivalent collapses to zero and the Bellman argmax reverses. Aggregation is now a method on the certainty equivalent with exactly one call site in the engine. `QuasiArithmeticMean.aggregate` is the generic `g⁻¹(Σ w · g(v))`; `PowerMean.aggregate` overrides it with an anchored weight/deviation log form that stays finite wherever the mathematical power mean is and keeps the geometric-mean limit exact arbitrarily close to `risk_aversion = 1`. `Q_and_F` hands over the whole joint lottery in one piece — every stochastic node of every reachable target, weighted by its regime-transition probability — instead of reducing per target and inverting at the end. Flattening before aggregating is what lets the power mean anchor the transform; the transform is still applied before every expectation, over stochastic state transitions and over regime transitions alike. `get_Q_and_F` and `get_compute_intermediates` share one `_get_compute_E_next_V` builder, so the Bellman Q and the NaN diagnostics cannot disagree. The linear path keeps its existing accumulation and arithmetic unchanged. Closes #412. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rKUdSzs1PU2aM41bACUkS
Member
Author
|
Closing: opened against |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## reachability #413 +/- ##
===============================================
Coverage ? 90.62%
===============================================
Files ? 171
Lines ? 15481
Branches ? 0
===============================================
Hits ? 14030
Misses ? 1451
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Benchmark comparison (main → HEAD)Comparing
|
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 #412.
Targets
reachability, notmain.The defect
With a
PowerMeancertainty equivalent,Q_and_Fbuilt the continuation asinverse(Σ w · transform(v))— per-targettransform, weighted reduction,one final
inverse. Forrisk_aversion > 1and small positive continuationvalues, the intermediate
v^(1-ra)overflows the dtype before any summationhappens, so a finite certainty equivalent collapses to
0and the Bellmanargmax reverses.
On this branch
PowerMeanhad noaggregateat all — the whole stable pathhad to be written, not just dispatched to.
The repair
Aggregation is now a method on the certainty equivalent, and there is exactly
one call site in the engine.
QuasiArithmeticMean.aggregate(values=..., weights=..., params=...)is thegeneric
g⁻¹(Σ w · g(v)), unchanged in behaviour for user-suppliedtransform pairs.
PowerMean.aggregateoverrides it with an anchored weight/deviation logform,
log CE = a + [log(W) + log1p(E/W)] / (1-ra), which stays finitewherever the mathematical power mean is and keeps the geometric-mean limit
exact arbitrarily close to
risk_aversion = 1.Q_and_Fhands over the whole joint lottery in one piece — everystochastic node of every reachable target, each weighted by its
regime-transition probability — instead of reducing per target and
inverting at the end. Flattening before aggregating is what lets the power
mean anchor the transform; a per-target
transform -> reduce -> inversedecomposition structurally cannot. The ordering constraint is preserved:
the transform is still applied before every expectation, over stochastic
state transitions and over regime transitions alike.
get_Q_and_Fandget_compute_intermediatesnow share one_get_compute_E_next_Vbuilder, so the BellmanQand the NaN diagnosticscannot disagree by construction. This removes ~140 lines of duplicated
per-target setup.
certainty_equivalent=None) path keeps its existingaccumulation and arithmetic exactly — no heavy exact path is imposed on
expected-utility models.
resolve_certainty_equivalentnow returns one arg-to-flat-name mappinginstead of separate transform/inverse mappings, since
aggregatereceives theunion and each callable takes the subset its own signature declares.
Evidence ingested
Both attachments on the issue: the round-4 archive excerpts inlined in the
body and
issue412-gridsearch-powermean-evidence.zipfrom the comment(reproducer, oracle, mutation family, action-reversal matrix, logs,
REPAIR-ACCEPTANCE.md,disposition.yaml, F6/R4 finding JSONs).The packet's own reproducer is signature-bound to the audited DC-EGM snapshot
(
5b4339e2) — it callsget_Q_and_F(..., scalar_targets=...), which does notexist on
reachability. Its substance has therefore been ported into theproject's own suite rather than vendored: the decimal oracle, the pinned
witnesses, the seven-case risk/scale family, and the acceptance list.
Acceptance coverage
Every item of
REPAIR-ACCEPTANCE.md:(1e-8, 2e-8)@ ra 8test_power_mean_regime_lottery_stays_finite_in_float32(1e-50, 2e-50)@ ra 8test_power_mean_regime_lottery_stays_finite_in_float64test_solved_values_are_equivariant_to_rescaling_the_model(stochastic health nodes),..._matches_reference_at_float{32,64}_scalestest_bellman_prefers_the_higher_certainty_equivalent_at_any_scale(4 float64 cases) +..._matches_reference_at_float32_scales(3 float32 cases)alive× thedeadtargettest_power_mean_aggregate_is_homogeneous_of_degree_one, both rescaling teststest_power_mean_aggregate_is_geometric_mean_at_unit_risk_aversion,..._is_continuous_around_unit_risk_aversion(ra = 1 ± 1e-6)test_power_mean_aggregate_is_linear_expectation_at_zero_risk_aversion, existingtest_zero_risk_aversion_reduces_to_expected_utilitytest_power_mean_aggregate_drops_zero_weight_entriestest_diagnostic_intermediates_reproduce_the_Bellman_Q,test_simulated_consumption_is_equivariant_to_rescaling_the_modelAll values are checked against a 120-digit
decimaloracle (the packet'sstable_power_mean, reimplemented intests/test_certainty_equivalent.py),under JIT, in both precisions.
Each new test was confirmed red on the pre-fix source before the repair
landed.
Resource gate
Epstein–Zin example, 25 periods, 400 × 2 wealth-health × 600 consumption,
float64, CPU. Shared box with a concurrent test run, so cold-compile numbers
are noisy; the ranges below are min/max over two runs each.
The linear path is unchanged within noise. The power-mean path costs roughly
+12% warm runtime and a longer cold compile — the anchored form emits
log/min/max/expm1/log1pwhere the naive route emitted a singlepow— and peak RSS drops slightly, since one concatenated reductionreplaces a per-target reduction chain.
Not addressed
Provenance.
PROVENANCE-AND-SCOPE.mdnames PR #395 as the likely first-badcommit but marks it
hypothesis_only; no bisect was run here. The repair isindependent of which commit introduced the seam.