Compare the FEVD the way the paper defines it, and stop a NaN that silently voids the posterior - #13
Merged
Conversation
…lently voids the posterior Four findings, in descending order of how much they change what this repo says about itself. **The headline replication statistic was not the paper's.** Three compounding problems. The `fevd` column `h` holds the (h+1)-step variance, so "one year" was reading five quarters, not four. The shares were formed by summing per-shock posterior *medians* and renormalising to 100%, but the medians sum to ~0.62 (GDP) / ~0.71 (CPI), so the renormalisation inflated every identified share by about a third. And the paper's Figure 4 "decomposes the mean of the sum of the decomposition", with the note attributing the gap to 100% to the unidentified shocks — its shares are posterior *means* of shares of *total* variance, renormalised over nothing. Its text is explicit that the identified shocks leave ~20% unexplained. Computed the paper's way, at 4 quarters, per-draw group share: | | here | paper | |---|---|---| | UK GDP, global | 37.4% [23.4, 52.2] 68% | ~40% | | UK CPI, global | 42.3% [25.5, 60.3] 68% | ~50% | | UK GDP, domestic | 38.9% | ~40% | | UK CPI, domestic | 36.6% | ~40% | The GDP match is real and the total explained share (~77%/~79% against the paper's ~80%) lines up. The CPI match was not: 49.5% against ~50% was produced by the renormalisation step, and on the paper's own definition this replication falls about 8pp short. `summary.md` now prints both tables and says which one is comparable, rather than printing the flattering one alone. The 68% band is ±14pp wide — wider than the discrepancy being discussed. **A latent bug that voids a whole run without failing.** A degenerate volume element made `log_importance_weight` return ±inf; one infinite log-weight makes `lw - lw.max()` NaN for *every* draw, so the weighted posterior is destroyed silently. It is rejected and counted now. `log_marginal_likelihood` likewise let NaN through `min(...) <= 0`. **The forecast evaluation was evaluating a model nobody publishes.** `rolling_origin_evaluation` has always accepted `dummies`; the runner never passed them. So the skill table described a BVAR with 2020Q2 in the likelihood as an ordinary observation, while every published chart comes from the dummied model. Under the published spec, UK GDP against a random walk with drift goes 1.064 -> 0.987 (h1), 1.124 -> 0.946 (h8), and ex-Covid h1 RMSE falls 21%. The dummies are real-time safe: a dummy column for a quarter after the origin is identically zero in training, which is now a test. CPI barely moves — that weakness is not a Covid artefact. **But improved is not skilful.** The evaluation runs 64 Diebold-Mariano tests per benchmark and has always noted it needs a multiplicity adjustment without supplying one. Added Benjamini-Hochberg: against the random walk with drift, min q = 0.40. Nothing survives at FDR 10%, in either specification, including Bank Rate — whose p = 0.018 is one of 64. Also: the coverage factors applied to published fans were measured on a model estimated *without* Covid dummies while every published fan has them, so the production-spec measurement is now computed alongside (UK GDP 68% h1 wants 0.86, not the applied 0.64); factors quoted to four decimals come from 49 overlapping origins with a ±6.7pp standard error, now recorded with its derivation; `run_replication.py` had no `--seed`, so the committed `summary.md` was not reproducible. Tests: +22, none removed. `test_fevd_headline_aggregates_equal_component_sums` asserted `x == x` and could not fail. The negative findings are pinned deliberately so they cannot be quietly lost. 115 -> 137 passed, and the 24 warnings are gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Four findings, in descending order of how much they change what this repo
says about itself.
The headline replication statistic was not the paper's. Three
compounding problems. The
fevdcolumnhholds the (h+1)-stepvariance, so "one year" was reading five quarters, not four. The shares
were formed by summing per-shock posterior medians and renormalising
to 100%, but the medians sum to ~0.62 (GDP) / ~0.71 (CPI), so the
renormalisation inflated every identified share by about a third. And
the paper's Figure 4 "decomposes the mean of the sum of the
decomposition", with the note attributing the gap to 100% to the
unidentified shocks — its shares are posterior means of shares of
total variance, renormalised over nothing. Its text is explicit that
the identified shocks leave ~20% unexplained.
Computed the paper's way, at 4 quarters, per-draw group share:
The GDP match is real and the total explained share (~77%/~79% against
the paper's ~80%) lines up. The CPI match was not: 49.5% against ~50%
was produced by the renormalisation step, and on the paper's own
definition this replication falls about 8pp short.
summary.mdnowprints both tables and says which one is comparable, rather than
printing the flattering one alone. The 68% band is ±14pp wide — wider
than the discrepancy being discussed.
A latent bug that voids a whole run without failing. A degenerate
volume element made
log_importance_weightreturn ±inf; one infinitelog-weight makes
lw - lw.max()NaN for every draw, so the weightedposterior is destroyed silently. It is rejected and counted now.
log_marginal_likelihoodlikewise let NaN throughmin(...) <= 0.The forecast evaluation was evaluating a model nobody publishes.
rolling_origin_evaluationhas always accepteddummies; the runnernever passed them. So the skill table described a BVAR with 2020Q2 in
the likelihood as an ordinary observation, while every published chart
comes from the dummied model. Under the published spec, UK GDP against
a random walk with drift goes 1.064 -> 0.987 (h1), 1.124 -> 0.946 (h8),
and ex-Covid h1 RMSE falls 21%. The dummies are real-time safe: a dummy
column for a quarter after the origin is identically zero in training,
which is now a test. CPI barely moves — that weakness is not a Covid
artefact.
But improved is not skilful. The evaluation runs 64 Diebold-Mariano
tests per benchmark and has always noted it needs a multiplicity
adjustment without supplying one. Added Benjamini-Hochberg: against the
random walk with drift, min q = 0.40. Nothing survives at FDR 10%, in
either specification, including Bank Rate — whose p = 0.018 is one of
64.
Also: the coverage factors applied to published fans were measured on a
model estimated without Covid dummies while every published fan has
them, so the production-spec measurement is now computed alongside
(UK GDP 68% h1 wants 0.86, not the applied 0.64); factors quoted to four
decimals come from 49 overlapping origins with a ±6.7pp standard error,
now recorded with its derivation;
run_replication.pyhad no--seed,so the committed
summary.mdwas not reproducible.Tests: +22, none removed.
test_fevd_headline_aggregates_equal_component_sumsasserted
x == xand could not fail. The negative findings are pinneddeliberately so they cannot be quietly lost. 115 -> 137 passed, and the
24 warnings are gone.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
🤖 Generated with Claude Code