Fix sampler portability and interval labels (for #703)#975
Fix sampler portability and interval labels (for #703)#975daimon-pymclabs wants to merge 1 commit into
Conversation
…CausalImpact Addresses two review items on #703: - Cell 14: replace the nutpie/jax/FAST_COMPILE sampler (not in the default install, and the kwarg was spelled `nut_sampler` rather than PyMC's `nuts_sampler`, so it was silently ignored) with the default NUTS sampler and modest sample_kwargs (draws=500, tune=500, target_accept=0.95). Keeps docs runnable for readers on a default CausalPy install. - Cells 28/29: the CausalImpact band uses tfp-causalimpact's equal-tailed pointwise interval, not an HDI. Label it "95% interval" while the genuine az.hdi bands for the CausalPy methods stay "95% HDI", and clarify the caption. Outputs for the affected cells are cleared — they need regenerating in an env with pymc + tfp-causalimpact since the docs build runs with nb_execution_mode="off". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
PR SummaryLow Risk Overview The ITS and synthetic-control fits now use default PyMC NUTS with shared, modest On the three-panel effect plot, CausalImpact’s band is labeled “95% interval” (equal-tailed pointwise bounds from Outputs for the refit cells, comparison figure, and MAE table were cleared because the author could not execute the notebook; those cells need to be re-run before merge so committed outputs match the new sampler and labels (ITS/SC numbers and quoted ≈ values may shift slightly). Reviewed by Cursor Bugbot for commit 583f593. Bugbot is set up for automated code reviews on this repo. Configure here. |
2 participants
Targets
cetagostini/causalimpact_v_causalpy(the #703 branch) to address the two outstanding "should fix before merge" items from the latest review round.Changes
1. Portable sampler (cell 14). The notebook used
nutpie+ thejaxbackend +mode="FAST_COMPILE", none of which ship with a default CausalPy install — so the notebook errored for most readers running it locally. There was also a typo: the key wasnut_sampler, but PyMC's argument isnuts_sampler(cf.iv_vs_priors.ipynb), so it was silently ignored rather than selecting nutpie. Replaced with the default NUTS sampler and modestsample_kwargs(draws=500, tune=500, target_accept=0.95), consistent with AGENTS.md asking docs to minimise MCMC load.2. Interval labelling (cells 28/29). The CausalPy ITS and SC bands compute a genuine
az.hdi(...), but the CausalImpact band comes fromtfp-causalimpact'spoint_effects_lower/upper, which is an equal-tailed pointwise interval — yet the shared legend and caption called all three "95% HDI". The figure now labels CausalImpact's band "95% interval" (CausalPy bands stay "95% HDI"), and the caption is clarified.The docs build runs with
nb_execution_mode = "off", so cell outputs are committed by hand. I changed the sampler and the figure, but I could not execute the notebook in my environment (nopymc/tfp-causalimpact). I therefore cleared the stale outputs for the cells whose results change — the ITS fit (14–15), the SC fit and summaries (23, 24, 26, 27), the three-panel figure (29), and the MAE table (31).@cetagostini — please re-run those cells in your env to repopulate outputs. Note the new default sampler will shift the ITS/SC numbers slightly, so the approximate figures quoted in the markdown (≈ −5.2 for ITS, ≈ −1.98 for SC, and the ~14×/~46× MAE ratios in cell 32) may need a light touch-up to match the regenerated run.
Everything else from the review round was already addressed on the branch. Happy to fold these commits directly into #703 instead of as a separate PR if that's easier.