Skip to content

Add Chapter 2 Documentation: Extending Hierarchical Model to Correlated Biomarkers#129

Open
Kwan-Jenny wants to merge 16 commits into
mainfrom
correlated-biomarkers-qmd
Open

Add Chapter 2 Documentation: Extending Hierarchical Model to Correlated Biomarkers#129
Kwan-Jenny wants to merge 16 commits into
mainfrom
correlated-biomarkers-qmd

Conversation

@Kwan-Jenny
Copy link
Copy Markdown
Collaborator

This PR adds a new Quarto (.qmd) presentation file titled "Extending the Hierarchical Model for Antibody Kinetics" to the serodynamics documentation.
Key features:

  • Model Focus:

    • Incorporates feedback from Dr. Morrison and Dr. Aiemjoy to focus exclusively on the Teunis (2016) within-host antibody kinetics model.

    • Clarifies model dynamics, parameter definitions, and the distinction between estimated vs. derived quantities.

    • Updates notation for consistency across project documentation.

  • Covariance Structure Extension:

    • Reviews current block-diagonal covariance assumption (Σ_K ⊗ I_J) that treats biomarkers as independent.

    • Proposes extended model with full biomarker covariance matrix (Σ_K ⊗ Σ_J) to capture correlation across antigen–isotype combinations.

    • Includes Kronecker product examples and expanded matrix forms for clarity.

  • Simulation & Validation Plan:

    • Outlines steps to simulate individuals with correlated biomarkers.

    • Compares independence model (I_J) vs. correlated model (Σ_J) via DIC/WAIC and posterior predictive checks.

    • Forms the basis for Chapter 2’s methodological framework and simulation study.

  • Next Steps:

    • Implement full Σ_J prior in JAGS.

    • Run simulation experiments to test model recovery of known correlation.

    • Apply extended model to Shigella longitudinal dataset once validated.

This .qmd file will serve as both a reproducible Beamer slide deck and internal reference for model development in Chapter 2.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 13, 2025

📖 https://ucd-serg.github.io/serodynamics/preview/pr129
Preview documentation for this PR (at commit 9c82e87)

@Kwan-Jenny
Copy link
Copy Markdown
Collaborator Author

Hi @d-morrison — as we talked about in our last meeting, could you review the notation here and share any overall feedback on this .qmd file? Thanks!

@Kwan-Jenny Kwan-Jenny requested a review from d-morrison August 18, 2025 22:06
Comment thread NEWS.md

## New features

* Added Chapter 2 documentation on correlated biomarker model (#129)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Added Chapter 2 documentation on correlated biomarker model (#129)
* Added documentation on correlated biomarker model (#129)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@Kwan-Jenny I'm taking a look at this PR this afternoon and posted this edit early by accident; you can ignore it until I finish the full review)

Copy link
Copy Markdown
Member

@d-morrison d-morrison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kwan-Jenny coming along! see notes in docx version on Teams


## Overview

- Incorporates feedback from Dr. Morrison and Dr. Aiemjoy\
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove progress notes from document; comments like this should go into the PR description on Github instead

Comment thread NEWS.md

## New features

* Added Chapter 2 documentation on correlated biomarker model (#129)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@Kwan-Jenny I'm taking a look at this PR this afternoon and posted this edit early by accident; you can ignore it until I finish the full review)

@Kwan-Jenny
Copy link
Copy Markdown
Collaborator Author

@copilot,
could you review my conversation above with d-morrison and update my files accordingly? In particular, please revise and enhance chapter2_correlated_biomarkers.qmd based on the images and feedback suggested by d-morrison. Additionally, please use the attached images and the Word file (which d-morrison reviewed) to further revise chapter2_correlated_biomarkers.qmd.
chapter2_correlated_biomarkers.docx

image image image image image image image

Also, please review the .qmd file thoroughly and carefully to improve it, and align the revisions with d-morrison’s approach and way of thinking.

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 31, 2026

@Kwan-Jenny I've opened a new pull request, #187, to work on those changes. Once the pull request is ready, I'll request review from you.

@Kwan-Jenny
Copy link
Copy Markdown
Collaborator Author

@copilot resolve the merge conflicts in this pull request

Copilot AI and others added 2 commits May 21, 2026 18:15
Co-authored-by: Kwan-Jenny <68584166+Kwan-Jenny@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 21, 2026 18:18
@Kwan-Jenny Kwan-Jenny review requested due to automatic review settings May 21, 2026 18:18
Copy link
Copy Markdown
Contributor

Copilot AI commented May 21, 2026

@copilot,
could you review my conversation above with d-morrison and update my files accordingly? In particular, please revise and enhance chapter2_correlated_biomarkers.qmd based on the images and ...

Updated vignettes/articles/chapter2_correlated_biomarkers.qmd using the available docx feedback and notation notes from Dr. Morrison. The revision is in commit 7a2c55e (notation cleanup, removal of progress-note phrasing, simplified Teunis mapping section, and clarified transformed-parameter assumptions).

@copilot resolve the merge conflicts in this pull request

Merge conflicts are resolved by merging origin/main into this branch and committing the conflict resolution in b54b54f.

@Kwan-Jenny
Copy link
Copy Markdown
Collaborator Author

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 21, 2026

Claude finished @Kwan-Jenny's task in 2m 56s —— View job


Claude PR Review

  • Read repo CLAUDE.md / copilot-instructions and project conventions
  • Read the changed files in this PR
  • Analyze notation, model derivations, and structure
  • Cross-check feedback already addressed (Morrison's notes)
  • Post review feedback

Nice work — the document reads well as both a slide deck and reference. The exposition in §1–§6 of the "Intuition: Full Walkthrough" is especially good for setting up the next-person/population distinction. Below are the issues I found, ordered by severity.

Critical (math / model correctness)

1. Kronecker product is written in the wrong order

vignettes/articles/chapter2_correlated_biomarkers.qmd:553-555, 617-625, 659-680, 731-740

The doc states

$$\text{Cov}(\text{vec}(\Theta_i)) = \Sigma_P \otimes I_B$$

and then expands it to a block-diagonal matrix with Σ_P on the diagonal:

[ Σ_P   0    0  ]
[  0   Σ_P   0  ]
[  0    0   Σ_P ]

That block-diagonal form is I_B ⊗ Σ_P, not Σ_P ⊗ I_B. Under the standard vec (column-stacking) convention, if Θ_i ∈ ℝ^{P×B} is a matrix-normal sample with row-covariance Σ_P and column-covariance Σ_B, then

$$\text{vec}(\Theta_i) \sim \mathcal{N}(\text{vec}(M),; \Sigma_B \otimes \Sigma_P).$$

So the independence case is I_B ⊗ Σ_P and the correlated case is Σ_B ⊗ Σ_P. This is a real correctness issue — either flip the Kronecker order throughout, or swap the row/column roles of Σ_P and Σ_B (and re-derive the expansions to match). Worth picking a convention up front and stating which vec orientation you're using.

2. Σ_{P,j} vs Σ_P is used inconsistently and these are different models

qmd:28, 232, 304, 359, 569, 623, 695, 748-751

Two distinct objects appear under similar names:

  • Σ_{P,j} — a per-biomarker 5×5 parameter covariance (Overview, "Subject-Level Parameters", "Priors on Covariance", Practical To-Do List).
  • Σ_P (no j) — a single 5×5 parameter covariance shared across biomarkers, which is the only thing the Kronecker Σ_P ⊗ I_B / Σ_P ⊗ Σ_B formulation makes sense with.

These describe substantively different models. A reader trying to map the math to JAGS code would be unsure whether you mean one Σ_P shared across biomarkers (5×5, then Kronecker'd with Σ_B) or per-biomarker Σ_{P,j} (which can't be combined with a single Σ_B via a clean Kronecker). Pick one and apply it consistently across the Overview, hierarchical-prior section, and matrix-algebra section.

3. τ_j is overloaded

qmd:50-54, 386, 718-722

τ_j is defined as measurement precision for biomarker j (Gamma prior). Then in the Σ_B example, the entries are also written τ_{11}, τ_{12}, …. Rename the Σ_B entries (e.g., ω_{kl} or σ_{B,kl}) to avoid collision with the precision parameter.

4. PR description vs document notation

The PR body uses Σ_K ⊗ I_J / Σ_K ⊗ Σ_J, the document uses Σ_P ⊗ I_B / Σ_P ⊗ Σ_B. Choose one set of indices and update both.

5. Time/visit index is inconsistent

  • qmd:40: observations use k for visit/time (y_{obs,ijk}).
  • qmd:411-413: observations use for visit (x_{iℓj}).

Same quantity, two indices — pick one.

Important (clarity / faithfulness to Morrison's feedback)

6. Morrison asked to remove "progress notes" — the "Practical To-Do List" section is the main offender

qmd:744-767

Morrison's review comment on the previous version was specifically: "remove progress notes from document; comments like this should go into the PR description on Github instead." The "Practical To-Do List (for Chapter 2)" section reads exactly like the planning notes that comment was aimed at. Consider either (a) moving it verbatim into the PR description, or (b) reframing it as a "Roadmap" subsection of the discussion in narrative form rather than a bulleted to-do list.

7. Parameter-counting story contradicts itself

  • qmd:77: "Only the first 6 are typically estimated" — but the table (qmd:65-74) lists 7 rows, of which t_1 and y_1 are derived, not estimated.
  • qmd:197-205: now claims 7 estimated parameters (5 core + 2 initial conditions), 1 derived (y_1).
  • qmd:268-281: again says 7 estimated, 1 derived.

The "6 estimated" claim on line 77 conflicts with the rest of the document. Fix it to: 5 core kinetic params (γ_y, γ_b, κ, α, ρ) + 2 initial conditions (y_0, b_0) = 7 estimated; t_1, y_1 derived.

8. Rendering / typo

  • qmd:177: \theta\_{ij} has an extra backslash; this will render as literal _ rather than a subscript. Should be \theta_{ij}.
  • qmd:393: two consecutive horizontal-rule --- lines back-to-back (one is redundant).

9. Beamer slide-level mismatch

The YAML sets slide-level: 2, but the long "Intuition: Full Walkthrough" section uses ### subheadings (qmd:397, 420, 462, 466, 492, 504, 514, 530). At slide-level 2, ### is content within a slide, not a new slide — so in the Beamer build those subsections will pile onto a single slide and almost certainly overflow. Either bump that walkthrough to ## slides or accept that it's an HTML-only section and gate it with :::{.content-visible when-format="html"}.

10. Inline lists won't render as Markdown lists

Several places put bullets immediately after a colon on the same line, e.g.:

  • qmd:401: **Level A — population (hyper-parameters):** - Mean vector across people: $\mu_j$ - Between-person covariance ...
  • qmd:405-407, qmd:411, qmd:477-481, qmd:485-487, qmd:495-497, qmd:499-501, qmd:508-510.

These won't render as bullets; the - will appear inline. They need a newline before each list item and a blank line before the list. This is also called out in .github/copilot-instructions.md:560: "include a blank line before bullet/numbered lists."

Minor

11. Equation labels exist but are never referenced

{#eq-1}, {#eq-2}, {#eq-ode-system}, {#eq-t1}, {#eq-y1}, {#eq-10}, {#tbl-param-summary}, {#tbl-model-comparison} are all labeled but the prose never cites them with @eq-… or @tbl-…. Either drop the labels or actually cross-reference them — half-applied cross-refs are a maintenance trap. (See .github/copilot-instructions.md:561.)

12. NEWS.md item

NEWS.md:50 says "Added Chapter 2 documentation on correlated biomarker model (#129)". Morrison's accidental early suggestion was "Added documentation on correlated biomarker model (#129)". Either reads fine, but if you want to honor the explicit suggestion, drop "Chapter 2" since the user-facing artifact is just a vignette/article.

13. WORDLIST entry uses a curly apostrophe

inst/WORDLIST:108: serodynamic's uses U+2019 instead of ASCII '. Depending on how spelling::spell_check_package() normalizes the source, this may not actually match the document's apostrophe and the entry can be silently ineffective. Worth checking and replacing with ASCII.

14. Reference list is one item

vignettes/articles/references.bib only contains @teunis2016. The hierarchical-Bayesian / Kronecker material likely deserves at least one foundational citation (e.g., Dawid 1981 on matrix-normal; or a standard reference for Wishart priors on covariance). Not blocking, but a one-item .bib is a small smell.

15. Hyperprior choice notes

qmd:359-370: ν_j = 6 for a 5×5 Wishart is the minimum (ν ≥ p+1 = 6) for a proper, finite-mean prior. That's fine but worth a one-line note that this is intentionally near the lower bound to give a weakly informative prior, since some readers will see ν=6 and worry.

Things that look good

  • The §1–§6 "Intuition" walkthrough is a really clean exposition of where MCMC is needed vs where Monte Carlo on posterior draws suffices, and it correctly distinguishes the population (uncertainty in μ) vs new-person (adds Σ_P) predictive objects.
  • The notation table mapping Teunis (2016) → serodynamics (qmd:145-152) is a nice addition and addresses the spirit of Morrison's "review the notation" comment.
  • ODE → closed-form solution → t_1, y_1 derivation flow is clear and self-contained.

Suggested fix priority

If you want to fix these in one pass, I'd tackle them in this order: #1 (Kronecker order), #2 (Σ_P vs Σ_{P,j}), #5 (k vs ), #3 (τ overload), #6 (remove to-do list), #7 (param-count fix), #8 (typo + double ---), #9 (slide-level), #10 (inline-list rendering), then mop up the minors.

If you'd like, I can open a follow-up commit that applies fixes #3, #5, #7, #8, #10, and #12#13 (the mechanical ones), and leave #1, #2, #6, #9 for you and Dr. Morrison to settle since they involve modeling-choice or document-scope decisions. Just say the word.


· branch: correlated-biomarkers-qmd

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants