Make zarr traces equivalent to arrow traces - #335
Conversation
ee5d697 to
051630a
Compare
|
Real Margus again: |
A trace returned from a zarr_store diverged from the arrow backend in four ways that break downstream consumers: * the arviz-convention sample_stats attrs added in 7451919 were only set on the arrow path, so pymc's patch_nutpie_idata fails with KeyError: 'inference_library_settings' - the zarr backend is unusable through pm.sample without monkeypatching; * chain/draw exist as dimensions but carry no coordinate variables, so label-based selection (posterior.sel(draw=...)) raises KeyError; * dict-valued attrs serialize fine to zarr but have no HDF5 equivalent, so a later to_netcdf() of the tree fails with 'Object dtype has no native HDF5 equivalent'; * save_warmup and the store_* settings are ignored, so the trace keeps both warmup groups and ten per-draw, parameter-sized sampler stats (gradient, unconstrained_draw, mass_matrix_*, divergence_*, transformed_*) that the arrow backend drops. Measured on a small pymc model that is an 8x larger stored trace, and the ratio grows with the parameter count. Hoist the attrs and the skip-list into helpers used by both paths, label chain/draw as the arrow backend does, JSON-encode dict attrs the way inference_library_settings already is, and drop the stats and warmup groups the settings exclude. Coords and vars are assigned per node in place, so the tree stays lazy and keeps any children. The regression test asserts attr parity with arrow, that num_tune (the field pymc dereferences) round-trips, that label-based selection works, that the tree survives to_netcdf, and that both backends return the same groups and sampler stats. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
051630a to
14a35b3
Compare
|
Extended: found a fourth divergence while using this downstream — the zarr path also ignores |
|
Thanks, this look useful. I'm wondering though if we should move most of this to the nuts-rs repo, so that users can just load the zarr file manually later if they want, and still get the correct format. |
|
You're right — correcting my description. Six of the ten are zero-length and cost nothing; four ( So the store_* half is a shape bug, not a volume one, and fixing it in nuts-rs (write them zero-length like the other six) makes them free everywhere rather than only in zarr — which supports moving this there, along with the |
|
Took a look at nuts-rs — you're right that it belongs there, and the The mechanism: The natural hook is The warmup half I left alone: Happy to open that as a nuts-rs PR whenever you want it, and to trim this one down to just the attrs + JSON-encoding once the Rust side lands. |
What / why
A trace returned from a
zarr_store(added in #244, following the request in #171) currentlydiverges from the arrow backend in four ways that break downstream consumers:
Missing
sample_statsattrs. The arviz-convention attrs added in 7451919(
inference_library,inference_library_version,inference_library_settings) are onlyset on the arrow path. PyMC's
patch_nutpie_idatareads them, sofails with
KeyError: 'inference_library_settings'— the zarr backend is unusable throughPyMC without monkeypatching.
No
chain/drawcoordinate variables. They exist as dimensions only, so label-basedselection (
posterior.sel(draw=...)) raisesKeyErroron zarr traces where it works onarrow ones.
Dict-valued attrs. Fine in zarr, but with no HDF5 equivalent, so a later
to_netcdf()of the tree fails withTypeError: Object dtype has no native HDF5 equivalent.save_warmupand thestore_*settings are ignored. The trace keeps both warmup groupsand ten per-draw, parameter-sized stats (
gradient,unconstrained_draw,mass_matrix_*,divergence_*,transformed_*) that the arrow backend drops. On a small PyMC model that isan 8x larger stored trace, and the ratio grows with the parameter count — it inflates every
saved trace, every re-read, and the peak memory of writing one out.
The fix handles all four in the zarr branch of
_extract. The attrs and the skip-list are hoistedinto
_sample_stats_attrs()/_skipped_stats()helpers used by both paths, so the two backendscannot drift again;
chain/draware labelled0..n-1as on the arrow path; dict attrs areJSON-encoded the way
inference_library_settingsalready is; and the stats and warmup groups thesettings exclude are dropped. Coords and vars are assigned per node in place, so the tree stays
lazy and keeps any children (
tree[path] = datasetwould replace the node and discard them).Motivation/context: #171 asked for the zarr backend precisely so that "pymc would then be
able to simply load the trace from the underlying storage without much need to nutpie
wrappers" — this makes that hold in practice. It also matters for the memory threads (#233,
#265): streaming to a
zarr_storekeeps sampling-phase memory flat instead of accumulatingthe trace in RAM, but only if the resulting trace is actually consumable downstream.
Test plan
tests/test_pymc.py::test_zarr_store_sample_stats_attrs(new) — asserts attr paritybetween the zarr and arrow trace, that
num_tune(the field PyMC dereferences)round-trips, that
sel(draw=...)works, that the tree survivesto_netcdf(), and that bothbackends return the same groups and the same sampler stats.
pm.sample(..., zarr_store=...)completes with no shims on this branch.🤖 Generated with Claude Code