Update for Turing v0.48 - #703
Conversation
|
Preview the changes: https://turinglang.org/docs/pr-previews/703 |
There was a problem hiding this comment.
🟡 Changes recommended
Three documentation inaccuracies concerning PG resampling, discarded steps, and callback iteration semantics remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates documentation and dependencies for Turing v0.48, including a rewritten, executable AbstractMCMC interface guide.
Changes:
- Upgrades Turing and resolved dependencies.
- Modernizes sampling, threading, vectorisation, and chain documentation.
- Updates tutorials, links, and executable examples.
File summaries
| File | Review |
|---|---|
usage/vectorisation/index.qmd |
Adds validation and minibatching guidance. |
usage/varnamedtuple/index.qmd |
Documents symbol indexing. |
usage/threadsafe-evaluation/index.qmd |
Describes task-local accumulators. |
usage/submodels/index.qmd |
Adds Stan distribution integration. |
usage/sampling-options/index.qmd |
Updates sampling guidance. Nit: clarify that callbacks run only for retained samples. |
usage/sampler-visualisation/index.qmd |
Updates sampler descriptions. |
usage/mode-estimation/index.qmd |
Clarifies bound validation. |
usage/external-samplers/index.qmd |
Documents the current external-sampler API. |
tutorials/multinomial-logistic-regression/index.qmd |
Fixes the NUTS API link. |
tutorials/infinite-mixture-models/index.qmd |
Updates random-measure imports. |
tutorials/coin-flipping/index.qmd |
Updates documentation links. |
tutorials/bayesian-logistic-regression/index.qmd |
Fixes the NUTS API link. |
tutorials/bayesian-linear-regression/index.qmd |
Fixes the NUTS API link. |
Project.toml |
Requires Turing 0.48. |
Manifest.toml |
Refreshes resolved dependencies. |
faq/index.qmd |
Revises threading guidance. |
developers/inference/abstractmcmc-interface/index.qmd |
Rewrites the interface guide. Nit: correct the explanation of discard_initial. |
core-functionality/index.qmd |
Updates sampler and chain guidance. Nit: distinguish PG’s first-sweep resampling from later conditional sweeps. |
_quarto.yml |
Updates the displayed version. |
Review details
Suppressed comments (2)
core-functionality/index.qmd:570
- This overstates how
PGuses its configured resampling scheme. In Turing 0.48 the scheme applies only to PG's first, unconditional sweep; later conditional sweeps select ancestors categorically from the weights. Please distinguish that behavior so readers do not expectStratifiedResampler(or a custom scheme) to govern every PG sweep.
By default both samplers resample with the stratified scheme whenever the effective sample size drops below half the number of particles.
The scheme and the threshold can be chosen: the scheme types are `StratifiedResampler`, `SystematicResampler` and `MultinomialResampler`, all in `Turing.Inference`, and the threshold is a fraction of the particle count.
developers/inference/abstractmcmc-interface/index.qmd:166
discard_initialadds discarded steps before collectingN; it does not remove 2,000 entries from the requested 20,000. The current wording can make readers expect an 18,000-element result, while this call returns 20,000 transitions after running 2,000 initial discarded steps.
`sample` runs the two methods, discards the first 2000 draws, and returns the remaining transitions as a vector.
- Files reviewed: 17/19 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
The broad documentation update and unresolved callback-accuracy nit warrant final human review.
Review details
Suppressed comments (1)
usage/sampling-options/index.qmd:278
- This overstates the callback behavior:
num_warmuponly selectsstep_warmupand does not itself discard samples, so retained warmup samples (for example, whendiscard_initial < num_warmup) do invoke the callback. Also, Turing v0.48's customSMCsampling path explicitly ignores callbacks. Please document both exceptions so users do not silently miss expected callback invocations.
The `callback` keyword argument can be used to specify a function that is called each time a sample is kept.
This function should have the signature `callback(rng, model, sampler, sample, state, iteration::Int; kwargs...)`, where `state` is the sampler state that produced the sample and `iteration` counts kept samples from 1 to the number requested.
Steps that are discarded through `num_warmup`, `discard_initial` or `thinning` do not trigger the callback.
- Files reviewed: 17/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
I left a few minor comments above; otherwise, it looks good. |
|
@yebai Applied all your suggestions and few more fixes. |
The interface guide described the AbstractMCMC v1 interface and its code cells were
eval: false, so CI never noticed. It is rewritten on the current interface with every cell executed.The
to_distributionexample is not executed because it needs BridgeStan and a Stan toolchain.