Stay in xarray in do() producers; simplify with native broadcasting#357
Stay in xarray in do() producers; simplify with native broadcasting#357drbenvincent wants to merge 3 commits into
Conversation
Replace numpy extract-and-re-wrap paths in run_do_pymc and run_do_panel_unified with xarray slice/mean/transpose operations. Remove _da_chain_draw* helpers and use public draws() at API boundaries. Closes #356 Co-authored-by: Cursor <[email protected]>
Replace the single-use numpy-rewrap helpers with broadcasting and named reductions so producers read as label algebra. Constants become `ones * value` off a coord-carrying template; the mean path inlines inverse-link/subgroup/mean; the predictive path just renames the obs dim to `unit`. Drops `_const_*`, `_select_subgroup`, `_mean_to_chain_draw`, `_predictive_to_chain_draw`, `_panel_to_time_chain_draw`, `_chain_draw_coords`, and the dead numpy branch of `_apply_inverse_link`. Net 79 lines smaller than main with no numpy bridges left in producers. Co-authored-by: Cursor <[email protected]>
The (n_times,) intervention path through run_do_panel_unified had no coverage; every panel simulate_over='time' test used a scalar set value. Add characterization tests asserting the intervened variable's per-time draws track the supplied array exactly and the outcome stays finite. Co-authored-by: Cursor <[email protected]>
|
@BugBot review |
PR SummaryMedium Risk Overview Removed the old New panel tests assert array-valued Reviewed by Cursor Bugbot for commit 858bfb9. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 858bfb9. Configure here.
Summary
Closes #356.
The
do()producers (run_do_pymc,run_do_panel_unified) now assemble labelledxr.DataArrays end-to-end. The leftover.to_numpy()→ reshape →_da_chain_draw*round-trips from the flat-dict era are gone, and so is the adapter machinery that existed only to re-wrap numpy back into labelled arrays.Rather than swap the old numpy wrappers for new single-use ones, the producers lean on xarray's native broadcasting and named reductions so they read as label algebra:
ones = _chain_draw_ones(post)turns every intervention/exogenous fill intoones * value— no manualdims/coordsornp.full/np.broadcast_to.inverse_link → isel(subgroup) → .mean(obs_dims); the g-computation standardization is one expression.unit;_stack_sampleflattens it at the public.draws()boundary._panel_unit_mean(average units →(time, chain, draw)) and_spread_over_timefor scalar/per-time interventions._apply_inverse_linkdropped its dead numpy branch and dispatchesnp.exp/pt.expon the input type, operating directly onDataArrays (noxr.apply_ufunc).Helpers removed:
_da_chain_draw,_da_chain_draw_unit,_da_time_chain_draw,_const_chain_draw,_const_time_chain_draw,_select_subgroup,_mean_to_chain_draw,_predictive_to_chain_draw,_panel_to_time_chain_draw,_extra_dims,_chain_draw_coords. Kept/added only small label-only helpers used across multiple call sites:_obs_dims,_chain_draw_ones,_spread_over_time,_panel_unit_mean,_fill_missing_posterior_rvs,_predictive_source,_exog_value.Small cleanups:
_model.pyprob()/sensitivity()use public.draws(); repr sample counts come from dim sizes.Size:
simulate.pyis 79 lines smaller thanmain(985 vs 1064), with no numpy bridges remaining in the producers.Test plan
uv run pytest tests/test_xarray_storage.py tests/test_estimand_result_api.py tests/test_reprs.py tests/test_panel_by_time.py tests/test_causal_queries.py tests/test_att_atu.pymake test: all pass, 87.8% coveragemake lint(ruff, ruff-format, mypy, license checks).to_numpy()/ numpy re-wrap on PyMC/ArviZ outputs in producers(n_times,)setpath throughrun_do_panel_unifiedwas previously exercised only with scalar values. New tests assert the intervened variable's per-time draws track the supplied array exactly.Notes for reviewers
test_bernoulli.py/test_families.py._panel_unit_meanand the predictive path identify the observation dims positionally (obs[0]= time,obs[1]= unit) because the scan compiler names them generically (mu_sales_dim_0/_dim_1). This(time, unit)ordering was confirmed against a live scan model; if the compiler ever changes it, this is the spot that would need a coord-name lookup instead.