Conversation
…sts + Meridian ROI priors New interop module converting DiD experiment results into Marketing Mix Model calibration inputs, with zero MMM-package dependency: - to_pymc_marketing_lift_test(): emits the lift-test DataFrame consumed by pymc_marketing MMM.add_lift_test_measurements (channel/dims/x/delta_x/ delta_y/sigma; explicit unit contract with aggregate= mean/sum scaling, n_units= geo-to-national scaling, on_wrong_sign= policy mirroring the upstream monotonicity check, x+delta_x>=0 post-test spend guard, reserved- column dims validation; prophetverse-compatible schema) - to_meridian_roi_prior(): emits Google Meridian lognormal ROI prior params (MeridianROIPrior with mu/sigma via Google's closed form - bit-identical parity vs meridian prior_distribution.lognormal_dist_from_mean_std at 1.7.0; spend-weighted multi-experiment pooling per the Meridian FAQ recommendation; se_widening transferability knob; channel-scoped .to_code() snippet - roi_m is per-channel so vector priors require the model channel order, scalar output needs explicit single_channel=True) - Duck-typed against the flat att/se surface every headline results class exposes; treated counts are never inferred from result metadata (n_treated is a row count on several classes); n_units/n_periods must be positive integers - 51 behavioral tests incl. units-not-rows pin, scipy lognormal round-trip, channel-scope regression, and cross-estimator contract (DiD/SDID/MPD/CS) - End-to-end smoke verified against real pymc-marketing 0.19.4 (frame accepted into the model graph; wrong-signed row raises NonMonotonicError) - Docs: REGISTRY.md interop section, api/mmm.rst, llms.txt + llms-full.txt, README one-liner, references.rst, doc-deps.yaml, CHANGELOG, TODO follow-ups Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wAAYSVo1Yf3XMcxNMrqsj
Overall AssessmentExecutive Summary
MethodologyP3 — Documented Meridian pooled-SD policy Impact: Concrete fix: None required. No unmitigated methodology findings PyMC-Marketing’s lift-test API consumes Code QualityP1 — Inline inference computation in new test helper Location: Impact: The new helper constructs Concrete fix: import from diff_diff.utils import safe_inference
t_stat, p_value, conf_int = safe_inference(att, se)
return DiDResults(
att=att,
se=se,
t_stat=t_stat,
p_value=p_value,
conf_int=conf_int,
...
)PerformanceNo findings. The exporters are small linear passes over experiment rows; no obvious performance issue is introduced. MaintainabilityNo blocking findings. The new module is cohesive, has explicit validation helpers, and keeps MMM package dependencies out of runtime imports. Tech DebtP3 — Deferred MMM interop items are properly tracked Impact: Missing Meridian Concrete fix: None required for this PR. SecurityNo findings. I did not see secrets, credentials, or unsafe external execution paths in the changed code. Documentation/TestsP3 — Meridian Location: Impact: Meridian docs note that JAX users need Concrete fix: Optional: add a Test execution I attempted to run Path to Approval
|
…substrate note in to_code() - P1: tests/test_mmm.py make_result() now builds t_stat/p_value/conf_int via safe_inference(att, se) instead of inline computation, honoring the project-wide all-or-nothing NaN inference contract - P3: .to_code() snippets note they use the TensorFlow substrate of TFP and that JAX-backed Meridian users should import tensorflow_probability.substrates.jax instead (generated-code comment + docstring) 51 tests green; black/ruff/mypy clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wAAYSVo1Yf3XMcxNMrqsj
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
MethodologyP3 — Documented Meridian pooled-SD policy Severity: P3 informational Impact: Concrete fix: None required. No unmitigated methodology findings PyMC-Marketing documents a lift-test frame with Code QualityNo findings. Re-review note: the previous P1 inline inference anti-pattern is resolved at PerformanceNo findings. The exporters are linear over the result rows and do not introduce heavy runtime dependencies. MaintainabilityNo findings. The new module is cohesive and keeps PyMC-Marketing/Meridian as output contracts rather than runtime dependencies. Tech DebtP3 — Deferred MMM interop follow-ups are tracked Severity: P3 informational Impact: Meridian Concrete fix: None required for this PR. SecurityNo findings. I did not see secrets, credential handling, unsafe subprocess execution, or network calls in the changed code. Documentation/TestsNo blocking findings. The new tests cover schema emission, scaling, wrong-sign policy, empty/drop-all handling, NaN/SE rejection, count handling, lognormal moment round-trip, pooling, channel-scoped code generation, and cross-estimator duck typing. Test execution attempted: Result: not run because |
Summary
diff_diff/mmm.pyconverting DiD experiment results into MMM calibration inputs, with zero MMM-package dependency (pure numpy/pandas; emits plain DataFrames/dataclasses)to_pymc_marketing_lift_test(): emits the lift-test DataFrame consumed by PyMC-Marketing'sMMM.add_lift_test_measurements(channel/dims/x/delta_x/delta_y/sigma), with an explicit four-column unit contract (aggregate="mean"|"sum"),n_units=geo-to-national scaling,on_wrong_sign={"raise","drop","keep"}policy mirroring the upstream monotonicity check, anx + delta_x >= 0post-test-spend guard, and reserved-columndimsvalidation; schema is prophetverse-compatibleto_meridian_roi_prior(): emits Google Meridian lognormal ROI prior parameters (MeridianROIPrior) via Google's closed form, with spend-weighted multi-experiment pooling (per the Meridian FAQ recommendation, citing sec 3.4 of Google's MMM calibration whitepaper), anse_widening=transferability knob, and a channel-scoped.to_code()snippet helper (roi_mis per-channel in Meridian, so vector priors require the model channel order; scalar output needs explicitsingle_channel=True)att/sesurface every headline results class exposes; treated counts are never inferred from result metadata (n_treatedis a row count on several classes);n_units/n_periodsmust be positive integersdocs/api/mmm.rst, llms.txt + llms-full.txt sections, README one-liner, references.rst entries, doc-deps.yaml entry, CHANGELOG, TODO.md follow-up rows for documented deferrals (Meridianroi_calibration_periodmask, event-study temporal lift export, Robyn, GeoX)Methodology references (required if estimator / math changes)
meridian/model/prior_distribution.pyat 1.7.0 (lognormal closed form + per-channelroi_mbatch shape + defaultLogNormal(0.2, 0.9)); Meridian FAQ (spend-weighted average ROI for multiple experiments, citing sec 3.4 of Google's MMM calibration whitepaper); Zhou, Choe & Hetrakul (2023), Meta Marketing Science (calibration motivation)roi_sd = sqrt(sum((w_i * sd_i)^2))treats experiments as independent - this uncertainty propagation is this library's policy (the FAQ prescribes only the spend-weighted mean); documented with a**Note:**in REGISTRY.md, withse_widening=as the correlated-experiments mitigation.Validation
tests/test_mmm.py(new, 51 behavioral tests): exact schema emission, hand-computed scaling for mean/sum/n_units, wrong-sign policies incl. drop-all-raises, go-dark boundary (x + delta_x == 0passes, below raises), NaN/zero-SE rejection, broadcasting/empty-sequence/reserved-dims errors, scipy lognormal moment round-trip (1e-12), spend-weighted pooling math, channel-scope regression for.to_code()(experiment prior lands only in its channel slot), units-not-rows pin (no silentn_treatedfallback), and cross-estimator duck-typing contract (DifferenceInDifferences / SyntheticDiD / MultiPeriodDiD / CallawaySantAnna)MMM.add_lift_test_measurementsinto the model graph, and a wrong-signed row reproduced upstreamNonMonotonicError;mu/sigmaverified bit-identical to Meridian'slognormal_dist_from_mean_stdover 200 random draws. Tutorial notebook follows in PR-B.Security / privacy
🤖 Generated with Claude Code
https://claude.ai/code/session_014wAAYSVo1Yf3XMcxNMrqsj