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. |
The zarr writer stores every variable it is handed, including each free RV's transformed value var. The arrow path pops those out via reparameterized_names and only resurfaces them, as unconstrained_posterior, when store_unconstrained is set; the zarr path returned the tree verbatim, so store_unconstrained meant different things on the two backends. Use the same reparameterized_names list rather than a name-suffix heuristic, so the two paths cannot disagree about what counts as unconstrained.
What / why
A trace returned from a
zarr_store(added in #244, following the request in #171) currentlydiverges from the arrow backend in five 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.
Transformed variables stay in
posterior. The zarr trace keeps every unconstrainedvalue variable (
sigma_log__, ...) in itsposteriorgroup; the arrow trace does not. On oneproduction model that is 174 posterior variables against arrow's 116 — 58 extras, at full draw
width and carrying auto-generated dimension names that downstream consumers don't expect.
Both backends are handed the same list:
_make_functionsalways includes each free RV'stransformed value-var name in
all_names/shape_info[0], andvar_namesonly prunesremaining_rvs, never the value vars — confirmed withreturn_raw_trace=True, where the rawarrow trace contains
sigma_log__too. nuts-rs stores what it is told, identically for both(
arrow.rsandzarr/sync_impl.rscall the samesettings.data_types(math)). The differenceis created afterwards: the arrow branch calls
_arrow_to_arviz, which popsreparameterized_namesand only resurfaces them, asunconstrained_posterior, whenstore_unconstrained=True; the zarr branch returned the tree verbatim.The fix handles all five 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; and the unconstrained value
variables are moved out of
posteriorusingreparameterized_names, the same list the arrow pathuses, so
store_unconstrainedmeans the same thing on both backends. Coords and vars are assigned per node in place, so the tree stayslazy 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.
One behavioural note: the arrow path adds
warmup_unconstrained_posterioreven whensave_warmup=False. The zarr path here handles the unconstrained groups after dropping warmup,so it does not — that looked like the intended behaviour rather than something to mirror, but say
the word and I'll match arrow exactly instead.
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.
tests/test_pymc.py::test_zarr_store_transformed_variables(new) — asserts that aHalfNormal'ssigma_log__is absent fromposterioron both backends by default, and thatwith
store_unconstrained=Trueboth put the same variables inunconstrained_posterior.pm.sample(..., zarr_store=...)completes with no shims on this branch.🤖 Generated with Claude Code