Meta #426: MMM generative workflows as stress-test for panel + simulate APIs - #468
Meta #426: MMM generative workflows as stress-test for panel + simulate APIs#468drbenvincent wants to merge 10 commits into
Conversation
…nally (#434) Phase 1: draw correlated residuals per block from chol_{block} (packed LKJCholeskyCov form); block members propagate as mu deterministics to descendants, mirroring estimation. Phase 2: hsgp basis coefficients are ordinary free RVs handled by the existing pm.do/draw path. Both NotImplementedError guards removed; round-trip tests added. Reviewed (round 1): APPROVE; taste notes applied (docstring propagation semantics, zero-noise smooth check in hsgp test). Verification: uv run pytest tests/test_simulate_data.py tests/test_hsgp_compile.py tests/test_residual_cov.py tests/test_hsgp_do.py -q => 45 passed.
Enumerates every free-RV name simulate() requires, with concrete shapes
(unit-indexed hierarchical params, packed chol_{block}, hsgp basis
weights) via the same zero-filled placeholder compile simulate() uses.
Clear ValueError when lag()-bearing specs omit panel=. Documented in
data_simulation.qmd.
Reviewed (round 1): APPROVE; taste notes applied (IntoFrame annotation,
hsgp docstring wording). Literal-valued transform args becoming RVs
flagged upstream as follow-up.
Verification: pytest tests/test_simulate_template.py
tests/test_public_api.py tests/test_simulate_data.py -q => green.
Cross-sectional draw path reuses the name new_columns after the scan branch; rename to new_columns_xs. No behavior change (51 tests green: simulate_data, residual_cov, stochastic_latent).
pooling={'intercept': True, 'slopes': [...], 'by_var': {'tv':
{'coefficient': ('geo',)}, 'theta_tv': 'none'}} pools individual
predictors/transform params over requested panel-dimension subsets
(hyperpriors mu_{var}_{dim}/sigma_{var}_{dim}); 'none' forces per-cell
parameters. Flat pooling dict unchanged; legacy RV names untouched.
Transform-leaf predictors are rejected at parse time (pooling a raw
column under a transformed term would silently zero the transformed
signal); multi-equation and naming collisions validated early;
dim-less user prior overrides are re-dims'd to preserve per-cell
semantics in both compile paths. _warn_partial_pooling_intercept now
conditional per equation. Documented in 16-panel-data.qmd.
Reviewed: round 1 REQUEST_CHANGES (transform-leaf corruption, two
majors) -> fixed; round 2 APPROVE, no findings.
Verification: test_pooling_dims 22 passed incl. slow MCMC recovery
(mu_tv_geo [1.478,2.990] vs truth [1.5,3.0]); pooled suites 95 passed;
simulate/template 36 passed; ruff + mypy clean.
mmm_transform_data fixture, the adstock-decay recovery DGP, and mmm_awareness.qmd Part 1 now generate ground truth via simulate() (panel=, pooling=, latent AR(1) with estimated init). New fast TestSimulateFunnelDGP: x2~x1 / x3~x1 / x4~x2+x3 / y ~ logistic_saturation(adstock(x4)) with zero-noise NumPy-reference equality derived from the params dict. data_simulation.qmd callout cross-links the MMM examples; freeze caches refreshed for both edited pages. Reviewed: round 1 REQUEST_CHANGES (orphaned old-cell tail in mmm_awareness.qmd + stale _freeze) -> fixed; round 2 APPROVE. Verification: test_v03_smoke + test_simulate_data + test_panel_latent green incl. slow classes.
Resolved conflicts across panel.py, compile.py, priors.py, _model.py, __init__.py, transforms.py, and docs/user_guide/18-panel-data.qmd. Key integration decisions: - build_panel_info keeps main's signature and validation (#388: null / duplicate / balance checks, require_rectangular flag); multi-D composite unit keys from #430 materialize via the new attach_composite_unit() helper called by model()/simulate()/ simulate_params_template(). - by_var pooling machinery (#431) re-applied on top of main's MuSpec intercept rework (#417): pooled predictors excluded from flat beta in both compile paths; contributions added to mu_gen AND mu_est. - Main's auto-drop of redundant partial-pooling intercepts supersedes the per-equation warning added for #431 (deleted upstream with its caller); tests updated to the new semantics. - transforms.py: kept main's normalize-aware apply_pymc panel branch (supersedes #428's deletion of _apply_pymc_panel). - Docs: our sections merged into renamed 18-panel-data.qmd with main's new link paths; freeze cache refreshed at main's new _freeze/docs/ layout. Verification on merged tree: 1862 passed / 2 skipped, coverage 90.90%.
Adversarial reviewReviewed at branch head Ordered by severity. Six blockers, then four consistency issues, then the test-matrix gaps. BlockersB1.
|
| # | Missing case | Nearest existing test |
|---|---|---|
| B1 | ~~ block member as a predictor of a downstream equation in simulate() |
test_residual_cov_round_trip — block members are terminal nodes |
| B2 | {"coefficient": (d1, d2)} with two or more dims |
all 11 coefficient entries in test_pooling_dims.py are 1-tuples |
| B3 | ScalingFactors reused on a reordered / subset / extended frame |
test_scaling.py only round-trips within the fitting frame |
| B4 | unit column values containing the `" | "` separator |
| B5/B6 | scan structure assertions (which tensors are sequences vs carries; n_steps bound) |
none — scan wiring is only tested end-to-end via recovery |
| C4 | negative / NaN / inf scaling divisors | zero is tested; sign and finiteness are not |
The pattern worth naming: every one of these is a silent-wrong-answer path, not a crash path, and the recovery-style tests this PR adds are structurally blind to them because they assert "posterior contains truth" on a DGP the same code generated. A simulate() bug that corrupts the DGP corrupts the assertion identically and the test still passes. For B1 and B3 specifically, please add at least one test that checks a generated column against a hand-written NumPy reference rather than against a pathmc-generated one — #428 already establishes exactly this pattern for transforms and it is the right pattern here too.
What I'd merge
The simulate() panel/transform work (#427/#428), simulate_params_template (#429), multi-dim panel keys (#430) modulo B4, single-dim by_var pooling (#431 — verified working: per-cell slopes 1.0/5.0/20.0 recovered to 3 decimal places through simulate()), and the latent AR work (#88) all look solid and are well tested.
Blocking merge: B1 (restore a scoped guard at minimum), B2 (fix or explicitly reject rank-N dims), B3 (unit-keyed factors), B4 (separator validation), B5 (resolve the dead scan block one way or the other), B6 (restore n_steps).
Happy to pair on any of these, or to split B1(b) and B2's preferred fix into their own issues if you'd rather land the guards here and do the real fixes next.
Verified against 21da9df in a clean pathmc-dev environment; all repro snippets above were executed, and the quoted outputs are actual, not illustrative.
Closes #427, closes #428, closes #429, closes #430, closes #431, closes #432, closes #433. Implements Phase 1 of #88 (tracked separately there). Meta: #426.
Summary
MMM/funnel generative workflows as an integration stress-test for pathmc's panel + simulate APIs. One commit-checkpoint per issue, each reviewed in-loop; full run report with per-issue evidence is on the tracking issue.
simulate()stacksimulate()gainspanel=/pooling=forwarded tocompile_to_pymc()likemodel(). Scan-compiled specs forward-simulate in one draw over thepm.do-fixed model; stochastic carry states are reconstructed asmu + sigma*innovationsso simulated chains stay DAG-consistent for simulate-and-recover.simulate()(zero-noise match vs a NumPy reference using pathmc's actual saturation formula, atol 1e-8); unreachableAdstock._apply_pymc_paneldeleted.pathmc.simulate_params_template(spec, data, panel=, pooling=, families=, latent=)returns{name: {kind, shape, dtype}}from the same zero-filled placeholder compilesimulate()uses.~~blocks draw correlated residuals from packedchol_{block}; cross-sectionalhsgp()terms flow through the existingpm.dopath. BothNotImplementedErrorguards removed (panel combos stay rejected).Panel expressiveness
panel={"unit": ["geo", "brand"], "time": "week"}builds a composite unit key (North|Acme) and pools over its Cartesian product; rectangularity validated with actionable errors; public results keep the flat(chain, draw, unit)dims contract.pooling={"by_var": {"tv": {"coefficient": ("geo",)}, "theta_tv": "none"}}pools individual predictors/transform params over requested dimension subsets (mu_{var}_{dim}/sigma_{var}_{dim}hyperpriors) or forces per-cell params. Flat pooling dict unchanged. Transform-leaf predictors rejected at parse time (pooling raw columns under transformed terms would silently zero the media signal).pathmc.Scaling(max / mean / fixed / divide-by-grid methods, per-panel-dimension groups, xarray/dict grids aligned by dim name). Estimation runs on scaled columns;simulate()inverts generated endogenous columns back to business units.predict()/do()stay in scaled units this phase (documented; escape hatchmodel.fitted_scaling).lag()term get estimatedinit_{var}initial conditions seeding the scan carry; newPathModel.latent_trajectory(var)returns posterior latent states as xarray(chain, draw, time, unit).Docs/tests
mmm_transform_data, the adstock-decay recovery DGP, andmmm_awareness.qmdPart 1 now generate ground truth viapathmc.simulate()instead of hand-written NumPy loops; new funnel-DGP integration test (x2 ~ x1 / x3 ~ x1 / x4 ~ x2 + x3 / y ~ logistic_saturation(adstock(x4))). Freeze caches refreshed for both edited pages.Verification
mainbefore the run:make test-fast= 1160 passed / 0 failed (tagpre-run/426-2026-08-24).make test= 1448 passed / 0 failed, coverage 90.35%;make check_lintclean (ruff + mypy);uv run great-docs buildsucceeds.Review notes
Each checkpoint went through an inline review loop; three real defects were caught and fixed with regression tests (endo-column init leak in #427, transform-leaf pooling corruption in #431,
_constructionregression breakingrefute_placeboin #432). Taste-level decisions are recorded as## Decision (2026-08-24)sections on the sub-issues; the ones worth a human eye are listed in the run report on #426 — notably thatpredict()/do()outputs remain in scaled units under #432.Known follow-ups (not addressed here): pre-existing
do(set={latent_var})KeyError for stochastic-latent panels; literal transform args becoming RVs named'0.7'; temporal-dynamics predicate duplicated across modules.