Error informatively on partial specification of a multivariate variable (#2239)#1434
Open
yebai wants to merge 9 commits into
Open
Error informatively on partial specification of a multivariate variable (#2239)#1434yebai wants to merge 9 commits into
yebai wants to merge 9 commits into
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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1434 +/- ##
==========================================
+ Coverage 81.64% 81.69% +0.05%
==========================================
Files 50 50
Lines 3579 3595 +16
==========================================
+ Hits 2922 2937 +15
- Misses 657 658 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
DynamicPPL.jl documentation for PR #1434 is available at: |
Contributor
Benchmarks @ 4a9e25aPerformance 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 7763 64-Core Processor WORD_SIZE: 64 LLVM: libLLVM-16.0.6 (ORCJIT, znver3) 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>
yebai
changed the title
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>
Member
Author
|
@sunxd3, can you help review this? |
Member
|
Yes! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.