Skip to content

add intro to mcmc techniques for Bayesian inference#837

Open
github-actions[bot] wants to merge 5 commits into
mainfrom
claude/issue-836-20260527-185948
Open

add intro to mcmc techniques for Bayesian inference#837
github-actions[bot] wants to merge 5 commits into
mainfrom
claude/issue-836-20260527-185948

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Draft PR opened by @claude to address #836.

Triggered by workflow run.

Addresses #836.

Adapts content from Dobson & Barnett 4e Chapter 13. Covers Monte Carlo
integration, the Metropolis-Hastings algorithm, the Gibbs sampler, burn-in,
convergence diagnostics (trace plots, ACF, Gelman-Rubin), and posterior
inference from MCMC samples, with worked R examples using the Bernoulli model.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 27, 2026

Claude finished @d-morrison's task in 3m 41s —— View job


Code Review — PR #837: Intro to MCMC Techniques

  • Gather context and read changed files
  • Review _subfiles/intro-bayes/_sec-mcmc.qmd
  • Review chapters/intro-bayes.qmd modifications
  • Check against CLAUDE.md conventions
  • Post findings

Summary

The new _sec-mcmc.qmd subfile adds a well-structured introduction to MCMC covering Monte Carlo integration, the Metropolis–Hastings algorithm, Gibbs sampling, burn-in, convergence diagnostics, and posterior inference. The content is accurate and the writing follows the prose style of the book. However, there are several issues ranging from a rendering-breaking undefined macro to convention violations that need to be addressed before this is ready to merge.


Critical

1. \bfy macro is undefined — will break rendering

\bfy is used 13 times throughout _sec-mcmc.qmd (e.g., lines 4, 13, 16, 24, 33, 36, 48, 92, 104, 111, 118, 124, 172), but no such macro exists in latex-macros/macros.qmd or anywhere else in the codebase. The defined macro for a bold/vector $y$ is \vy (defined as \vecf{y}).

Every occurrence of \bfy should be replaced with \vy.

Fix this →


Significant

2. fig-cap chunk option used instead of :::{#fig-...} div format

_sec-mcmc.qmd line 213 uses #| fig-cap: "MH chain trace plot; dashed line marks end of burn-in." as a chunk option, but CLAUDE.md requires:

Use div format (:::{#fig-...}) for figures and tables, not chunk-option fig-cap/tbl-cap

The trace plot chunk should be restructured as:

:::{#exm-trace-plot}
## Trace plot for MH chain

:::{#fig-trace-mh}
```{r}
#| code-fold: show
library(ggplot2)
burnin <- 500L
df_chain <- data.frame(iteration = seq_along(mh_chain), pi = mh_chain)
ggplot(df_chain, aes(x = iteration, y = pi)) +
  geom_line(linewidth = 0.2, alpha = 0.7) +
  geom_vline(xintercept = burnin, linetype = "dashed", colour = "red") +
  labs(x = "Iteration", y = expression(pi)) +
  theme_bw()

MH chain trace plot; dashed line marks end of burn-in.
:::

:::


[Fix this →](https://claude.ai/code?q=In%20%60_subfiles%2Fintro-bayes%2F_sec-mcmc.qmd%60%20on%20branch%20%60claude%2Fissue-836-20260527-185948%60%2C%20the%20trace%20plot%20example%20at%20lines%20207-223%20uses%20%60%23%7C%20fig-cap%3A%60%20as%20a%20chunk%20option.%20Per%20CLAUDE.md%20conventions%2C%20replace%20this%20with%20a%20%60%3A%3A%3A%7B%23fig-trace-mh%7D%60%20fenced%20div%20wrapping%20the%20code%20chunk%2C%20with%20the%20caption%20as%20plain%20text%20inside%20the%20div%20after%20the%20chunk.%20Remove%20the%20%60label%3A%20fig-trace-mh%60%20and%20%60fig-cap%3A%60%20chunk%20options.&repo=d-morrison/rme)

**3. Forward reference to `@sec-mcmc-diagnostics` in the burn-in section**

`_sec-mcmc.qmd` line 192:

The length of the burn-in period is chosen by inspection of trace plots
(see @sec-mcmc-diagnostics).


This references `{#sec-mcmc-diagnostics}`, which is not defined until line 195 (the very next section heading). CLAUDE.md states "Introduce concepts before using them — no forward references." Reorder to mention trace plots after introducing convergence diagnostics, or remove the forward cross-reference.

**4. Forward reference to `@sec-jags`**

`_sec-mcmc.qmd` line 177:

JAGS (Just Another Gibbs Sampler) automates this approach for a wide class of models;
see @sec-jags for an example.


`#sec-jags` is defined after the MCMC section in `chapters/intro-bayes.qmd` (line 112). This is a forward reference. Either remove it or restructure so the mention follows the JAGS section.

---

### Moderate

**5. `library(ggplot2)` loaded inside an example chunk**

`_sec-mcmc.qmd` line 215 calls `library(ggplot2)` inside the `#exm-trace-plot` chunk. Package loading should happen in a setup chunk at the top of the parent chapter, not buried inside an example. The parent `chapters/intro-bayes.qmd` already loads `ggplot2` earlier (lines 63, 92), but a consistent setup chunk is preferred.

**6. Implicit cross-chunk variable dependencies**

`burnin` (defined at line 215 inside `#exm-trace-plot`) is referenced in `#exm-post-summary-mh` at line 264. Similarly, `mh_chain` from `#exm-mh-bernoulli` (line 160) is used in `#exm-trace-plot` (line 216). These cross-chunk dependencies are implicit and could silently break if chunks are reordered. Consider either making each example self-contained or adding a brief comment noting the dependency.

**7. `\ndist{}` preferred over `\Normal(...)`**

`_sec-mcmc.qmd` line 136 uses `\Normal(0, \sigma^2)`, but the codebase defines `\ndist{0, \sigma^2}` (`= \Normal\paren{#1}`) which auto-sizes the parentheses. Prefer `\ndist{0, \sigma^2}` for consistency.

---

### Minor / Nits

**8. `\mathrm{Beta}` — no established macro**

`_sec-mcmc.qmd` line 48: `\mathrm{Beta}(56, 37)`. There is no `\Beta` macro in `macros.qmd`; all other distribution names use `\operatorname{...}` via defined macros (e.g., `\Ber`, `\Normal`, `\Pois`). Consider adding `\def\Beta{\operatorname{Beta}}` to `macros.qmd` for consistency.

**9. Acceptance ratio in §"Why the normalizing constant..." duplicates §"MH algorithm definition"**

The acceptance ratio formula is written out twice (lines 104–106 and lines 123–126). This is intentional (prose context vs. formal definition), which is fine, but make sure the two presentations are exactly consistent — the inline prose at lines 107–109 says "accept … with probability $\min(1, \alpha)$" while the definition at line 123 folds the `min` into the formula. The current structure works but could confuse readers who compare the two.

---

### `chapters/intro-bayes.qmd` modifications

The change adds five lines:

```qmd
# Markov Chain Monte Carlo (MCMC) {#sec-mcmc}

{{< include _subfiles/intro-bayes/_sec-mcmc.qmd >}}

and removes the blank line that preceded the JAGS section. This integration is correct: the section heading is in the parent (not the subfile), satisfying the subfile convention. No issues here beyond the forward reference noted in item 4 above.


Verdict

This PR should not be merged in its current state due to the undefined \bfy macro (issue 1) which will cause a rendering failure. Issues 2–4 are significant convention violations. Issues 5–9 are improvements that would raise the quality to match the rest of the book.

@github-actions github-actions Bot requested a review from d-morrison May 27, 2026 22:19
@d-morrison
Copy link
Copy Markdown
Owner

@claude address review comments

@github-actions
Copy link
Copy Markdown
Contributor Author

👀 Picked up by workflow run #26543221524. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@d-morrison d-morrison marked this pull request as ready for review May 28, 2026 03:37
@d-morrison
Copy link
Copy Markdown
Owner

@claude review

@github-actions github-actions Bot removed the request for review from d-morrison May 28, 2026 03:37
@github-actions
Copy link
Copy Markdown
Contributor Author

👀 Picked up by workflow run #26553161091. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 28, 2026

Claude finished @d-morrison's task in 2m 37s —— View job


Code Review — PR #837 (re-review after fixes)

  • Read changed files and recent commits
  • Check which issues from prior review were addressed
  • Review _subfiles/intro-bayes/_sec-mcmc.qmd
  • Review chapters/intro-bayes.qmd
  • Post findings

What Was Fixed

Commit 60ed663 addressed the following from the prior review:

  • \bfy\vy — all 13 occurrences replaced; rendering will no longer break
  • Forward reference to @sec-jags — removed from the Gibbs section
  • Forward reference to @sec-mcmc-diagnostics — removed from the burn-in section
  • library(ggplot2) inside example chunk — removed; ggplot2 is already loaded at the parent chapter level (line 63)
  • #| label: fig-trace-mh and #| fig-cap: chunk options — removed from trace plot chunk

Remaining Issues

Significant

1. Trace plot figure has no caption and no div wrapper

The fig-cap chunk option was correctly removed from #exm-trace-plot, but the fix stopped there. CLAUDE.md requires the div format for figures:

Use div format (:::{#fig-...}) for figures and tables, not chunk-option fig-cap/tbl-cap

The trace plot currently renders as an unlabeled, uncaptioned plot inside an example div — not suitable for a textbook. The chunk at _sec-mcmc.qmd lines 208–217 should be wrapped in a :::{#fig-trace-mh} div with the caption as plain text:

:::{#fig-trace-mh}
```{r}
#| code-fold: show
burnin <- 500L
df_chain <- data.frame(iteration = seq_along(mh_chain), pi = mh_chain)
ggplot(df_chain, aes(x = iteration, y = pi)) +
  geom_line(linewidth = 0.2, alpha = 0.7) +
  geom_vline(xintercept = burnin, linetype = "dashed", colour = "red") +
  labs(x = "Iteration", y = expression(pi)) +
  theme_bw()

MH chain trace plot; dashed line marks end of burn-in.
:::


[Fix this →](https://claude.ai/code?q=In%20%60_subfiles%2Fintro-bayes%2F_sec-mcmc.qmd%60%20on%20branch%20%60claude%2Fissue-836-20260527-185948%60%2C%20wrap%20the%20R%20code%20chunk%20inside%20%60%3A%3A%3A%7B%23exm-trace-plot%7D%60%20in%20a%20%60%3A%3A%3A%7B%23fig-trace-mh%7D%60%20fenced%20div%2C%20placing%20the%20caption%20text%20%22MH%20chain%20trace%20plot%3B%20dashed%20line%20marks%20end%20of%20burn-in.%22%20as%20plain%20text%20inside%20the%20div%20after%20the%20code%20chunk.%20Per%20CLAUDE.md%20conventions%2C%20figures%20must%20use%20div%20format%20not%20chunk%20options.&repo=d-morrison/rme)

---

**Moderate**

**2. `\Normal(0, \sigma^2)` — use `\ndist{}` instead**

`_sec-mcmc.qmd` line 136:

$\varepsilon \sim \Normal(0, \sigma^2)$

`\Normal` is defined as `\operatorname{N}` (bare operator, no parentheses). The macro `\ndist{0, \sigma^2}` wraps in auto-sized parentheses and should be preferred for consistency with the rest of the book.

[Fix this →](https://claude.ai/code?q=In%20%60_subfiles%2Fintro-bayes%2F_sec-mcmc.qmd%60%20on%20branch%20%60claude%2Fissue-836-20260527-185948%60%2C%20line%20136%3A%20replace%20%60%5CNormal(0%2C%20%5Csigma%5E2)%60%20with%20%60%5Cndist%7B0%2C%20%5Csigma%5E2%7D%60.%20The%20%60%5Cndist%7B%7D%60%20macro%20is%20defined%20in%20latex-macros%2Fmacros.qmd%20and%20uses%20auto-sized%20parentheses%3B%20%60%5CNormal%60%20is%20just%20the%20bare%20operator%20name.&repo=d-morrison/rme)

**3. `\mathrm{Beta}` — no established macro**

`_sec-mcmc.qmd` line 48:

$\pi \mid \vy \sim \mathrm{Beta}(56, 37)$

`\mathrm{Beta}` is raw LaTeX. No `\Beta` macro exists in `macros.qmd`. Other distributions use `\operatorname{...}` via defined macros (`\Ber`, `\Normal`, `\Pois`). Either add `\def\Beta{\operatorname{Beta}}` to `macros.qmd` and use `\Beta(56, 37)`, or use `\operatorname{Beta}(56, 37)` inline.

[Fix this →](https://claude.ai/code?q=In%20%60_subfiles%2Fintro-bayes%2F_sec-mcmc.qmd%60%20on%20branch%20%60claude%2Fissue-836-20260527-185948%60%20line%2048%2C%20replace%20%60%5Cmathrm%7BBeta%7D%60%20with%20%60%5CoperatorName%7BBeta%7D%60%20or%20add%20%60%5Cdef%5CBeta%7B%5CoperatorName%7BBeta%7D%7D%60%20to%20latex-macros%2Fmacros.qmd%20and%20use%20%60%5CBeta%60.%20%60%5Cmathrm%7BBeta%7D%60%20is%20raw%20LaTeX%20inconsistent%20with%20the%20book%27s%20macro%20conventions%20for%20distribution%20names.&repo=d-morrison/rme)

**4. Implicit cross-chunk variable dependencies**

`burnin` is defined inside `#exm-trace-plot` (line 210) but used later in `#exm-post-summary-mh` (line 259). Similarly, `mh_chain` from `#exm-mh-bernoulli` (line 160) is consumed in both `#exm-trace-plot` and `#exm-post-summary-mh`. These cross-example dependencies will silently break if chunks are reordered. Each example could either be made self-contained (redefining `burnin` inline in `#exm-post-summary-mh`) or the dependency documented with a brief note.

---

### `chapters/intro-bayes.qmd`

No new issues. The five-line addition (section heading + include) is correct and the section heading lives in the parent file, satisfying the subfile convention.

---

### Verdict

The critical rendering blocker (`\bfy`) is now fixed. The forward references and `library()` loading issues are resolved. Three moderate issues remain (figure div wrapper, `\ndist`, `\mathrm{Beta}`) plus the implicit cross-chunk dependencies. The PR can be reviewed further once the figure caption/label is restored in div format (issue 1 above) — the others are lower priority but should be addressed before merge.

@github-actions github-actions Bot requested a review from d-morrison May 28, 2026 03:40
@d-morrison
Copy link
Copy Markdown
Owner

@claude iterate

@github-actions
Copy link
Copy Markdown
Contributor Author

👀 Picked up by workflow run #26641130635. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that.

@github-actions github-actions Bot removed the request for review from d-morrison May 29, 2026 14:56
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 29, 2026

Claude finished @d-morrison's task in 2m 11s —— View job


Code Review — PR #837 (post-iterate, round 3)

  • Gather context and read changed files
  • Check which prior-review issues were addressed by the latest commits
  • Review _subfiles/intro-bayes/_sec-mcmc.qmd (current state)
  • Review chapters/intro-bayes.qmd
  • Post findings

What Was Fixed by the Iterate Commits

Commits 2f503ea and 9765de0 addressed all remaining items from the prior review:

  • Trace plot wrapped in :::{#fig-trace-mh} div with caption:::{#fig-trace-mh}::: now encloses the code chunk; caption text follows as plain text inside the div. Structure is correct per CLAUDE.md.
  • \Normal(0, \sigma^2)\ndist{0, \sigma^2} — line 136 now uses the auto-paren macro.
  • \mathrm{Beta}\operatorname{Beta} — line 48 fixed; no \Beta macro needed.
  • burnin cross-chunk dependencyburnin <- 500L is now re-declared at the top of #exm-post-summary-mh (line 262), so that example is self-contained.

One Remaining Minor Nit

\hat{\mu} — use \hmu macro

_sec-mcmc.qmd line 247:

- **Posterior mean**: $\hat{\mu} = \frac{1}{T - B} \sum_{t=B+1}^{T} \vth^{(t)}$

\hmu is defined in latex-macros/macros.qmd (line 215: \def\hmu{\hat{\mu}}). Use \hmu for consistency.

Fix this →


Structural Note (no action required)

mh_chain is still a cross-chunk variable: it is generated in #exm-mh-bernoulli (line 160) and consumed in #exm-trace-plot (line 212) and #exm-post-summary-mh (line 263). This is pedagogically intentional — each example builds on the previous one — and the dependency is clear from the narrative. No change is needed here.


Verdict

All critical and significant issues from prior reviews are resolved. The only remaining item is the low-priority \hat{\mu}\hmu nit. The PR is otherwise ready to merge.

@github-actions github-actions Bot requested a review from d-morrison May 29, 2026 14:58
@github-actions
Copy link
Copy Markdown
Contributor Author

PR Preview Action v1.8.1
🚀 Preview available at: https://d-morrison.github.io/rme/pr-preview/pr-837/

Built to branch gh-pages at 2026-05-29 15:12 UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant