Add documentation on partial specification of a multivariate variable (#2239)#1434
Conversation
A variable drawn from a multivariate distribution in a single tilde-statement (e.g. `x ~ MvNormal(...)` / `filldist`) is a single random variable, not i.i.d. components. `predict` therefore cannot fix a subset of its components while resampling the rest, and `fix` cannot fix them independently. Add warning admonitions documenting this, referencing TuringLang/Turing.jl#2239. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1434 +/- ##
=======================================
Coverage 81.64% 81.64%
=======================================
Files 50 50
Lines 3579 3579
=======================================
Hits 2922 2922
Misses 657 657 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
DynamicPPL.jl documentation for PR #1434 is available at: |
Benchmarks @ 762856bPerformance Ratio: gradient time divided by log-density time. For very small models these ratios are noisy across runs and machines; raw primal and gradient timings are more reliable. The benchmarks are aimed at DynamicPPL developers and mainly catch obvious allocation or type-stability regressions. See benchmark notes for details. Main @ d7e84ceEnvironmentJulia Version 1.11.9 Commit 53a02c0720c (2026-02-06 00:27 UTC) Build Info: Official https://julialang.org/ release Platform Info: OS: Linux (x86_64-linux-gnu) CPU: 4 × AMD EPYC 9V74 80-Core Processor WORD_SIZE: 64 LLVM: libLLVM-16.0.6 (ORCJIT, znver4) Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores) |
…s specified Supplying a subset of a variable that the model samples as a single multivariate draw (e.g. `x[1:10]` when the model draws `x ~ MvNormal(zeros(20), ...)`) has never worked: `predict` silently resampled the whole variable from the prior, `fix` silently collapsed it to the supplied length, and `condition` threw an opaque `DimensionMismatch`. This is therefore not a behaviour change — it only replaces those silent or confusing failures with a single, informative error. Add `_check_supplied_shape(dist, supplied, vn)`, dispatched on the supplied representation (a materialised value for `condition`/`fix`, or the parameter `VarNamedTuple` for `predict`/`InitFromParams`), which throws one clear error referencing TuringLang/Turing.jl#2239. Only multivariate distributions are checked; per-index (`x[i] ~ ...`) declarations and correctly-sized whole-variable values are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cover the #2239 misuse cases: predicting with a chain from a differently-sized model, and `fix`/`condition` of a single index of a multivariate variable, all now raise the informative error. A whole-variable `fix` of the correct size is kept as a positive control. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No behaviour change. Shorten the `_check_supplied_shape` docstring and the `init` comment, and reduce the predict misuse test to the smallest model that still triggers the error (just the multivariate variable, no extra latent or observation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document the new informative error for partial specification of a multivariate variable via condition/fix/predict (Turing#2239). Non-breaking: the case never worked, so this only replaces a silent or opaque failure with a clear message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The docstring notes previously described the old silent behaviour (predict resampling the whole variable from the prior); update them to state that supplying only part of a multivariate variable now raises an error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
predict and fixCo-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keep the error self-contained and actionable (the loop workaround); the #2239 reference stays in the docstrings and HISTORY. Tests now match on a stable phrase from the message rather than the issue number. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback: raise `ArgumentError` (consistent with `check_tilde_rhs` /`check_dot_tilde_rhs`) instead of a generic `error`, and match it by type in the tests rather than by a fragile message substring. Add a regression test that per-index (`.~`) variables still grow correctly under `predict` without being falsely flagged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@sunxd3, can you help review this? |
|
Yes! |
|
I'm not sure the macro-level guard is the right place for this check — it both misses cases and catches one it shouldn't. It only runs when On the other side, a conditioned value is an observation, and the observe pipeline accepts batches via @model mvc(n) = m ~ MvNormal(zeros(n), I)
# errors on this branch:
fix(mvc(3), Dict(@varname(m[1]) => 1.0))()
# silently ignored, m sampled from the prior:
fix(mvc(3), Dict(@varname(m[1]) => 1.0, @varname(m[3]) => 3.0))()
# two draws, summed by loglikelihood — works on main, errors on this branch:
loglikelihood(condition(mvc(3), (; m = [1.0 2.0; 3.0 4.0; 5.0 6.0])), (;))Getting condition/fix right would need the tilde pipeline to tell "absent" apart from "partially present", and to treat observe-side shapes differently from assume-side — that's on the hot path. Honestly not sure it's worth fixing at all; if it is, definitely its own PR. So maybe we scope this down: keep the PR documentation-only (note the whole-variable semantics in the (prepared with CC + Fable) |
|
Thanks, @sunxd3. It would be great to find a robust, principled solution that catches unintended uses of One approach that often works well is to brainstorm with coding agents: give them a clear design goal or constraint, ask them to explore feasible solutions, and then evaluate the trade-offs of their proposals. In a sense, the process resembles the Metropolis–Hastings accept/reject step in MCMC, with humans deciding which proposed solutions to keep. I'm happy to merge the docstring fixes first, as you proposed, and then have you take on the remaining changes in a separate PR. |
Revert the tilde-site guard, the 0.42.2 bump, and the error tests, per the PR discussion: the guard misses most partial cases (noncontiguous indices, matrix-variate, submodel prefixes) and rejects valid batch observations. Keep the docstring notes on whole-variable semantics for predict and fix. The behaviour-side check moves to a follow-up PR via InitFromParams.
predict: partial chain values mean the whole variable is silently resampled and the predictions look plausible while ignoring the chain. fix: a partial fix may silently collapse the variable or be ignored. condition: partial conditioning may abort with an unrelated DimensionMismatch or be silently ignored.
Fixes the documentation and the behaviour side of TuringLang/Turing.jl#2239.
A variable drawn from a multivariate distribution in a single tilde-statement (e.g.
x ~ MvNormal(...),filldist) is a single random variable, not a collection of i.i.d. components. Supplying only part of such a variable — viapredictwith a chain from a differently sized model, orfix/conditionon a subset of indices — should produce an informative error.Update: (sunxd3) scoped down to documentation only, per the discussion below — the tilde-site guard, version bump, and error tests are reverted; the docstring notes on whole-variable semantics for
predictandfixremain. The behaviour-side check forpredictwill follow in a separate PR viaInitFromParams; condition/fix enforcement (if worth doing) will get its own design issue.