Add live fit monitoring (streaming callbacks) to the course#46
Open
seabbs-bot wants to merge 5 commits into
Open
Add live fit monitoring (streaming callbacks) to the course#46seabbs-bot wants to merge 5 commits into
seabbs-bot wants to merge 5 commits into
Conversation
Surface the streaming-callback pattern from epiforecasts/BVDOutbreakSize in the course (issue sbfnk#45): - New reference page reference/live_monitoring.qmd showing the AbstractMCMC per-iteration callback hook, a dependency-free progress recorder that streams log-density and divergences from transition.stats, the warmup log-density climb, a Neal's funnel divergence example, and an optional TensorBoard backend to run locally. - mcmc.qmd: animate the hand-coded Metropolis-Hastings chain so students watch well-tuned vs sticky mixing unfold, plus a collapsed pointer to the reference page. - mcmc_diagnostics.qmd: collapsed note linking to the reference page from the divergent-transitions discussion.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds teaching materials demonstrating live streaming of MCMC diagnostics during sampling. A new reference page explains the AbstractMCMC ChangesMCMC Live Monitoring Reference and Examples
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
reference/live_monitoring.qmd (3)
107-109: 💤 Low valueBare
catchblock hides all exceptions.The catch block on lines 107-109 silently swallows all exceptions, including potential bugs in the callback logic itself. While the comment explains that "a progress callback must never bring down a fit," consider at least logging caught exceptions during development.
💡 Optional: Log exceptions for debugging
catch e - # A progress callback must never bring down a fit. + # A progress callback must never bring down a fit. + `@debug` "Progress callback error" exception=(e, catch_backtrace()) endThis preserves the fail-safe behaviour whilst enabling debugging with
ENV["JULIA_DEBUG"] = "Main".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reference/live_monitoring.qmd` around lines 107 - 109, Replace the bare "catch" that silently swallows all exceptions with a named exception handler (e.g., "catch e") and log the exception when debugging is enabled; specifically, in the block that wraps the progress callback (the catch surrounding the progress callback invocation in live_monitoring), change to "catch e" and call a logger (for example `@error` or print with string(e) and optionally stacktrace) guarded by ENV["JULIA_DEBUG"] == "Main" so normal runs keep the fail-safe behaviour but developers can see exceptions during debugging.
253-253: 💤 Low valueAdd a trailing newline at end of file.
Many text editors and POSIX standards expect files to end with a newline character.
📝 Proposed fix
- [Betancourt (2017), A Conceptual Introduction to Hamiltonian Monte Carlo](https://arxiv.org/abs/1701.02434) — why divergences signal geometric problems +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reference/live_monitoring.qmd` at line 253, The file reference/live_monitoring.qmd is missing a trailing newline; open the file and add a single newline character at the end of the file (ensure the final line ends with '\n'), save and commit so the file terminates with a newline as required by POSIX and common editors.
30-30: ⚡ Quick winClarify wording for the “companion proposals” references in reference/live_monitoring.qmd (line 30).
Line 30 mixes a repository link (
epiforecasts/BVDOutbreakSize) with an issue link (EpiBranch.jl#112), so “companion proposals” is slightly misleading; rephrase to make the resource types clear. Both links resolve successfully.Suggested change
-The companion proposals are [epiforecasts/BVDOutbreakSize](https://github.com/epiforecasts/BVDOutbreakSize) and [epiforecasts/EpiBranch.jl#112](https://github.com/epiforecasts/EpiBranch.jl/issues/112). +The companion resources are the [BVDOutbreakSize](https://github.com/epiforecasts/BVDOutbreakSize) repository and [EpiBranch.jl#112](https://github.com/epiforecasts/EpiBranch.jl/issues/112).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@reference/live_monitoring.qmd` at line 30, Change the phrase "companion proposals" to explicitly state the resource types: mention that one is a repository (epiforecasts/BVDOutbreakSize) and the other is an issue/discussion in EpiBranch.jl (EpiBranch.jl#112); update the sentence on line containing "The companion proposals are" to something like "Related resources: the repository epiforecasts/BVDOutbreakSize and issue EpiBranch.jl#112" so readers can tell one is a repo and the other is an issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@reference/live_monitoring.qmd`:
- Around line 107-109: Replace the bare "catch" that silently swallows all
exceptions with a named exception handler (e.g., "catch e") and log the
exception when debugging is enabled; specifically, in the block that wraps the
progress callback (the catch surrounding the progress callback invocation in
live_monitoring), change to "catch e" and call a logger (for example `@error` or
print with string(e) and optionally stacktrace) guarded by ENV["JULIA_DEBUG"] ==
"Main" so normal runs keep the fail-safe behaviour but developers can see
exceptions during debugging.
- Line 253: The file reference/live_monitoring.qmd is missing a trailing
newline; open the file and add a single newline character at the end of the file
(ensure the final line ends with '\n'), save and commit so the file terminates
with a newline as required by POSIX and common editors.
- Line 30: Change the phrase "companion proposals" to explicitly state the
resource types: mention that one is a repository (epiforecasts/BVDOutbreakSize)
and the other is an issue/discussion in EpiBranch.jl (EpiBranch.jl#112); update
the sentence on line containing "The companion proposals are" to something like
"Related resources: the repository epiforecasts/BVDOutbreakSize and issue
EpiBranch.jl#112" so readers can tell one is a repo and the other is an issue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 481791cf-3354-4294-bb89-e5723b0230b9
📒 Files selected for processing (3)
reference/live_monitoring.qmdsessions/mcmc.qmdsessions/mcmc_diagnostics.qmd
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
seabbs
reviewed
Jun 4, 2026
…ions Responds to PR review on the live-fit-monitoring page: - reference/live_monitoring.qmd: drop the BVD package-extension asides; rename the recorder section to "A simple progress recorder"; turn a code comment into prose; add a note on why the built-in mcmc_callback (AbstractMCMC >= 5.13) can't be used on the pinned Turing; add an optional example that animates the fit from inside the callback; rework the TensorBoard section to use the new shared helper, note it is reusable elsewhere (e.g. PMCMC), and that TensorBoard is also a handy viewer for completed fits; show multiple-chain run dirs instead of the single-chain caveat. - src/tensorboard_callback.jl: new opt-in TensorBoard streaming helper (loads TensorBoardLogger only when you ask for it), logging params and diagnostics as scalars and running histograms. - mcmc.qmd: trim the animation intro, fold the "good proposal is hard to find" motivation into the sticky-vs-tuned demo, and move the Turing callback pointer to after NUTS is introduced. - mcmc_diagnostics.qmd: reframe the note as an optional dashboard for watching inference in progress and reviewing results.
Author
|
Pushed a revision addressing all the review comments (replied inline and resolved each thread). Highlights:
Re-rendered This was opened by a bot. Please ping @seabbs for any questions. |
Bumps the inference stack to the latest Turing (0.45.0), which makes the
official AbstractMCMC mcmc_callback / ParamsWithStats interface work and
replaces MCMCChains with FlexiChains as the default chain type.
Dependency changes (Project.toml / Manifest.toml):
- Turing 0.42.2 -> 0.45.0 (DynamicPPL 0.41, AbstractMCMC 5.15).
- Add AbstractMCMC, TensorBoardLogger, FlexiChains as direct deps.
TensorBoard helper (src/tensorboard_callback.jl):
- Rewritten to read parameters and diagnostics through the official
AbstractMCMC.ParamsWithStats interface, logging grouped params/ and
diagnostics/ tags with scalar traces and running histograms (the
grouped-tabs dashboard from BVDOutbreakSize). Verified end-to-end.
Reference page (reference/live_monitoring.qmd):
- Drop the "mcmc_callback errors on the pinned Turing" caveat (now works);
present the built-in mcmc_callback one-liner alongside the grouped helper.
Session migration MCMCChains -> FlexiChains (verified by re-render):
- `using MCMCChains` -> `using FlexiChains`.
- Diagnostics: `DataFrame(rhat/ess/mcse/mean(chain))` -> one
`DataFrame(summarystats(chain))` (`:param`, `:rhat`, `:ess_bulk`, ...).
- Divergences: `chain.info[:divergent_transitions]` ->
`sum(skipmissing(vec(chain[:numerical_error])))`.
- Energy: `chain.info[:energy]` -> `vec(chain[:hamiltonian_energy])`.
- Per-chain `chain[:, :, i]` -> `chain[chain = i]`; `length` -> `size(.,1)`.
- Posterior predictive `predictions[Symbol("obs[$j]")]` ->
`predictions[@varname(obs[j])]`.
- `describe` -> `summarystats`; `corner` -> StatsPlots `cornerplot`.
Author
|
Heads up: this PR's scope grew. It now also bumps the course to Turing 0.45.0 and migrates from MCMCChains to FlexiChains (the new default chain type), so the live-monitoring feature can use the official What changed beyond the original live-monitoring page:
Validation: re-rendered the whole site fresh on 0.45. 8/10 sessions render clean with zero errors; the last two (seitl, universal_differential_equations) were still rendering on a heavily-loaded machine when this was pushed — I'll confirm/fix those in a follow-up commit if needed. This was opened by a bot. Please ping @seabbs for any questions. |
`rand(model)` now returns a `VarNamedTuple`, so `params.R_0` field access errored. Wrap with `NamedTuple(rand(model))` to restore field access.
Two breakages from the dependency bump (unrelated to FlexiChains): - Lux 1.29 + ForwardDiff no longer differentiates the neural-ODE loss (`Lux.PartialsFn` error). Switch training to reverse-mode `AutoZygote`, with SciMLSensitivity providing the adjoint through the ODE solve (added as a direct dep). Reverse-mode is the standard choice for neural-network training anyway; explanations updated to match. - `sol.retcode != :Success` is always true now that retcode is a `ReturnCode` enum, so `predict_ude` always returned NaN (constant loss -> nothing gradient -> `iterate(::Nothing)`). Use `SciMLBase.successful_retcode`.
Author
|
Migration complete — all 10 sessions now render clean on Turing 0.45 + FlexiChains (full fresh re-render, no errors). The two non-chain sessions that broke on the stack bump (not the FlexiChains migration) are fixed:
This was opened by a bot. Please ping @seabbs for any questions. |
Owner
|
@seabbs-bot can we split this in 2 PRs, one for the changes needed to work with latest Turing (and need to happen) from the live monitoring (less convinced all of this needs to be exposed to the course participants)? |
Collaborator
|
Sure happy to rebase this on an updated main. I think in general this flags issues from not having dependabot etc running across this. |
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
3 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.
Closes #45.
This surfaces the streaming-callback / TensorBoard pattern we use in epiforecasts/BVDOutbreakSize in the course, adapted to the Turing version pinned here (
transition.statsis read directly rather than through the newerAbstractMCMC.ParamsWithStatsconstructor).What and where
Issue #45 proposed two teaching ideas. This PR does both, keeping the sessions light by putting the substantive how-to on a reference page (matching the existing
reference/logdensity_interface.qmdpattern).New reference page
reference/live_monitoring.qmd— "Watching a fit unfold":callbackhook and whattransition.statsexposes for NUTS (log-density, divergence flag, step size, acceptance rate).progress_recorderclosure that streams the log-density and a running divergence count, wrapped intry/catchso it can never abort a fit.discard_adapt = false) to watch the log-density climb into the typical set.sessions/mcmc.qmd— the hand-coded Metropolis-Hastings idea from the issue: a small animated GIF showing a well-tuned chain vs a sticky (too-small-step) one filling in side by side, so students watch good vs bad mixing. Plus a collapsed pointer to the reference page.sessions/mcmc_diagnostics.qmd— a collapsed note linking to the reference page from the divergent-transitions discussion, framing live monitoring as the slow-model complement to the post-hoc checks.Notes
quarto render. The animation and all callback code execute headlessly (GKSwstype=100).eval: false).CONTRIBUTING.md(collapsed blue notes for optional context).This was opened by a bot. Please ping @seabbs for any questions.
Summary by CodeRabbit
Documentation