Add verbal plot descriptions to 18 tutorial notebooks for RAG ingestion - #204
Add verbal plot descriptions to 18 tutorial notebooks for RAG ingestion#204jonathanhhb wants to merge 17 commits into
Conversation
The tutorial notebooks under docs/tutorials/notebooks/ end with matplotlib
figures with no surrounding prose explaining what the figures show. When
these notebooks are fed to a documentation LLM / RAG (e.g. laser-mcp), the
model cannot read the rendered PNG outputs nearly as well as it can read
a pre-existing verbal summary of the axes, curves, and takeaway. This
commit adds that prose and the tooling to maintain it.
For each plot-bearing code cell, inserts one '### Reading the ... plot'
markdown cell directly after the cell, nested under the existing '##'
section heading. Each description follows an observation -> interpretation
-> application sequence:
- Observation: names the axes, color/marker mapping, curve shape, and
specific numeric landmarks (peak values, time-to-peak, asymptotes).
- Interpretation: what the curves mean in the modeling / epi context,
using the same domain vocabulary the notebook itself uses
(beta, R0, S, I, R, "logistic", "patch", "stochastic", etc.).
- Application: a single bolded sentence stating what the figure
DEMONSTRATES (sampler reproduces the analytic PDF, parameter
recovery within Monte-Carlo noise, CCS scales as alpha^1.5, etc.).
The bold lead-in is the RAG-friendly takeaway a chunker will key on.
Notebooks covered (18 total, 70 plot cells):
01_SI_nobirths_logistic_growth 4 plots
02_SI_wbirths_logistic_growth 4 plots
03_SIS_nobirths_logistic_growth 4 plots
04_SIR_nobirths_outbreak_size 3 plots
05_SIR_wbirths_age_distribution 5 plots
06_SIR_wbirths_natural_periodicity 2 plots
07_SIR_CCS 4 plots
08_2patch_SIR_wbirths_correlation 1 plot
09_rabies_diffusion_1D 3 plots
10_England_and_Wales 4 plots
SEI_and_SEIS_implementations 5 plots
births 2 plots
constant_pop 1 plot
distributions 10 plots
grid_examples 6 plots
mortality 3 plots
ri_exploration 4 plots
seasonality 6 plots
Excluded: EW_analysis.ipynb (no rendered plots), demographics/age_pyramid
and demographics/kmestimator (outputs cleared, distinct from the rendered
copies already described in the laser repo).
Adds tools/add_plot_descriptions.py, a manifest-driven helper that applies
these inserts idempotently. The manifest at tools/plot_descriptions/
config.json maps each notebook to a list of (after-cell marker, md-file)
pairs; the marker is a unique substring of the plot code cell, more durable
than a brittle cell index. The prose itself lives in standalone .md files
alongside the manifest so PR diffs are reviewable as plain markdown rather
than JSON-escaped strings.
The script supports --check for CI: exits non-zero if any notebook would
change, catching cases where someone edits a notebook and accidentally
drops a description cell. Marker matching is restricted to code cells so
the script remains idempotent even when an inserted description quotes
the marker string verbatim in its own prose.
No notebook code was modified; existing plot outputs are preserved.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds RAG-friendly, verbal “Reading the … plot” descriptions to plot-producing cells in the tutorial notebooks under docs/tutorials/notebooks/, along with a manifest-driven tool to (re)apply those inserts idempotently and support CI “drift” checking.
Changes:
- Adds
tools/add_plot_descriptions.pyto insert plot-description markdown cells after marker-matched code cells (with--checksupport). - Adds
tools/plot_descriptions/config.jsonplus many standalone.mdplot descriptions used by the inserter. - Updates the target tutorial
.ipynbfiles to include the inserted markdown description cells.
Reviewed changes
Copilot reviewed 90 out of 90 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/add_plot_descriptions.py | Manifest-driven script to insert/check plot-description markdown cells in notebooks |
| tools/plot_descriptions/config.json | Maps notebooks to marker strings and corresponding plot-description .md files |
| tools/plot_descriptions/01_SI_nobirths_beta_recovery.md | Plot description markdown (01_SI_nobirths_beta_recovery) |
| tools/plot_descriptions/01_SI_nobirths_sanity.md | Plot description markdown (01_SI_nobirths_sanity) |
| tools/plot_descriptions/01_SI_nobirths_t0_offset.md | Plot description markdown (01_SI_nobirths_t0_offset) |
| tools/plot_descriptions/02_SI_wbirths_logistic_growth_beta_cbr_recovery.md | Plot description markdown (02_SI_wbirths_logistic_growth_beta_cbr_recovery) |
| tools/plot_descriptions/02_SI_wbirths_logistic_growth_row0_trace.md | Plot description markdown (02_SI_wbirths_logistic_growth_row0_trace) |
| tools/plot_descriptions/02_SI_wbirths_logistic_growth_sanity.md | Plot description markdown (02_SI_wbirths_logistic_growth_sanity) |
| tools/plot_descriptions/02_SI_wbirths_logistic_growth_t0_offset.md | Plot description markdown (02_SI_wbirths_logistic_growth_t0_offset) |
| tools/plot_descriptions/03_SIS_nobirths_logistic_growth_beta_gamma_recovery.md | Plot description markdown (03_SIS_nobirths_logistic_growth_beta_gamma_recovery) |
| tools/plot_descriptions/03_SIS_nobirths_logistic_growth_gamma_dt_error.md | Plot description markdown (03_SIS_nobirths_logistic_growth_gamma_dt_error) |
| tools/plot_descriptions/03_SIS_nobirths_logistic_growth_sanity.md | Plot description markdown (03_SIS_nobirths_logistic_growth_sanity) |
| tools/plot_descriptions/03_SIS_nobirths_logistic_growth_t0_offset.md | Plot description markdown (03_SIS_nobirths_logistic_growth_t0_offset) |
| tools/plot_descriptions/04_SIR_nobirths_outbreak_size_compartments.md | Plot description markdown (04_SIR_nobirths_outbreak_size_compartments) |
| tools/plot_descriptions/04_SIR_nobirths_outbreak_size_final_size_sweep.md | Plot description markdown (04_SIR_nobirths_outbreak_size_final_size_sweep) |
| tools/plot_descriptions/04_SIR_nobirths_outbreak_size_seed_check.md | Plot description markdown (04_SIR_nobirths_outbreak_size_seed_check) |
| tools/plot_descriptions/05_SIR_wbirths_age_distribution_age_at_infection_fit.md | Plot description markdown (05_SIR_wbirths_age_distribution_age_at_infection_fit) |
| tools/plot_descriptions/05_SIR_wbirths_age_distribution_fraction_susceptible_by_age.md | Plot description markdown (05_SIR_wbirths_age_distribution_fraction_susceptible_by_age) |
| tools/plot_descriptions/05_SIR_wbirths_age_distribution_Iage_expected_vs_observed.md | Plot description markdown (05_SIR_wbirths_age_distribution_Iage_expected_vs_observed) |
| tools/plot_descriptions/05_SIR_wbirths_age_distribution_population_age_distribution.md | Plot description markdown (05_SIR_wbirths_age_distribution_population_age_distribution) |
| tools/plot_descriptions/05_SIR_wbirths_age_distribution_SIR_trajectory_and_age_histogram.md | Plot description markdown (05_SIR_wbirths_age_distribution_SIR_trajectory_and_age_histogram) |
| tools/plot_descriptions/06_SIR_wbirths_natural_periodicity_single_run_periodicity.md | Plot description markdown (06_SIR_wbirths_natural_periodicity_single_run_periodicity) |
| tools/plot_descriptions/06_SIR_wbirths_natural_periodicity_sweep_periodicity.md | Plot description markdown (06_SIR_wbirths_natural_periodicity_sweep_periodicity) |
| tools/plot_descriptions/07_SIR_CCS_alpha_loglog_fit.md | Plot description markdown (07_SIR_CCS_alpha_loglog_fit) |
| tools/plot_descriptions/07_SIR_CCS_fit_surface_3d.md | Plot description markdown (07_SIR_CCS_fit_surface_3d) |
| tools/plot_descriptions/07_SIR_CCS_patch_heatmap.md | Plot description markdown (07_SIR_CCS_patch_heatmap) |
| tools/plot_descriptions/07_SIR_CCS_vs_R0_and_alpha.md | Plot description markdown (07_SIR_CCS_vs_R0_and_alpha) |
| tools/plot_descriptions/08_2patch_SIR_wbirths_correlation_coupling_vs_correlation.md | Plot description markdown (08_2patch_SIR_wbirths_correlation_coupling_vs_correlation) |
| tools/plot_descriptions/09_rabies_diffusion_1D_relative_error_panels.md | Plot description markdown (09_rabies_diffusion_1D_relative_error_panels) |
| tools/plot_descriptions/09_rabies_diffusion_1D_wave_heatmap.md | Plot description markdown (09_rabies_diffusion_1D_wave_heatmap) |
| tools/plot_descriptions/09_rabies_diffusion_1D_wavespeed_scatter.md | Plot description markdown (09_rabies_diffusion_1D_wavespeed_scatter) |
| tools/plot_descriptions/10_England_and_Wales_best_combined.md | Plot description markdown (10_England_and_Wales_best_combined) |
| tools/plot_descriptions/10_England_and_Wales_ccs_grid.md | Plot description markdown (10_England_and_Wales_ccs_grid) |
| tools/plot_descriptions/10_England_and_Wales_london_phase_observed.md | Plot description markdown (10_England_and_Wales_london_phase_observed) |
| tools/plot_descriptions/10_England_and_Wales_top10_wavelet_phase.md | Plot description markdown (10_England_and_Wales_top10_wavelet_phase) |
| tools/plot_descriptions/births_constant_cbr.md | Plot description markdown (births_constant_cbr) |
| tools/plot_descriptions/births_time_varying_cbr.md | Plot description markdown (births_time_varying_cbr) |
| tools/plot_descriptions/constant_pop_seir_dynamics.md | Plot description markdown (constant_pop_seir_dynamics) |
| tools/plot_descriptions/distributions_beta.md | Plot description markdown (distributions_beta) |
| tools/plot_descriptions/distributions_binomial.md | Plot description markdown (distributions_binomial) |
| tools/plot_descriptions/distributions_exponential.md | Plot description markdown (distributions_exponential) |
| tools/plot_descriptions/distributions_gamma.md | Plot description markdown (distributions_gamma) |
| tools/plot_descriptions/distributions_logistic.md | Plot description markdown (distributions_logistic) |
| tools/plot_descriptions/distributions_lognormal.md | Plot description markdown (distributions_lognormal) |
| tools/plot_descriptions/distributions_normal.md | Plot description markdown (distributions_normal) |
| tools/plot_descriptions/distributions_poisson.md | Plot description markdown (distributions_poisson) |
| tools/plot_descriptions/distributions_uniform.md | Plot description markdown (distributions_uniform) |
| tools/plot_descriptions/distributions_weibull.md | Plot description markdown (distributions_weibull) |
| tools/plot_descriptions/grid_examples_central_city.md | Plot description markdown (grid_examples_central_city) |
| tools/plot_descriptions/grid_examples_hub_spoke.md | Plot description markdown (grid_examples_hub_spoke) |
| tools/plot_descriptions/grid_examples_infectious_snapshots.md | Plot description markdown (grid_examples_infectious_snapshots) |
| tools/plot_descriptions/grid_examples_linear.md | Plot description markdown (grid_examples_linear) |
| tools/plot_descriptions/grid_examples_ring.md | Plot description markdown (grid_examples_ring) |
| tools/plot_descriptions/grid_examples_seir_timeseries.md | Plot description markdown (grid_examples_seir_timeseries) |
| tools/plot_descriptions/mortality_cdr_single_run.md | Plot description markdown (mortality_cdr_single_run) |
| tools/plot_descriptions/mortality_cdr_sweep.md | Plot description markdown (mortality_cdr_sweep) |
| tools/plot_descriptions/mortality_estimator_single_run.md | Plot description markdown (mortality_estimator_single_run) |
| tools/plot_descriptions/ri_exploration_baseline_seir.md | Plot description markdown (ri_exploration_baseline_seir) |
| tools/plot_descriptions/ri_exploration_dose_timing_histogram.md | Plot description markdown (ri_exploration_dose_timing_histogram) |
| tools/plot_descriptions/ri_exploration_infectious_comparison.md | Plot description markdown (ri_exploration_infectious_comparison) |
| tools/plot_descriptions/ri_exploration_with_ri_seir.md | Plot description markdown (ri_exploration_with_ri_seir) |
| tools/plot_descriptions/seasonality_baseline_no_forcing.md | Plot description markdown (seasonality_baseline_no_forcing) |
| tools/plot_descriptions/seasonality_si_baseline.md | Plot description markdown (seasonality_si_baseline) |
| tools/plot_descriptions/seasonality_si_intervention.md | Plot description markdown (seasonality_si_intervention) |
| tools/plot_descriptions/seasonality_sinusoidal_no_importation.md | Plot description markdown (seasonality_sinusoidal_no_importation) |
| tools/plot_descriptions/seasonality_two_node_out_of_phase.md | Plot description markdown (seasonality_two_node_out_of_phase) |
| tools/plot_descriptions/seasonality_with_importation.md | Plot description markdown (seasonality_with_importation) |
| tools/plot_descriptions/SEI_and_SEIS_implementations_sei_suite.md | Plot description markdown (SEI_and_SEIS_implementations_sei_suite) |
| tools/plot_descriptions/SEI_and_SEIS_implementations_sei_vs_seis_mean.md | Plot description markdown (SEI_and_SEIS_implementations_sei_vs_seis_mean) |
| tools/plot_descriptions/SEI_and_SEIS_implementations_sei_vs_si_mean.md | Plot description markdown (SEI_and_SEIS_implementations_sei_vs_si_mean) |
| tools/plot_descriptions/SEI_and_SEIS_implementations_seis_suite.md | Plot description markdown (SEI_and_SEIS_implementations_seis_suite) |
| tools/plot_descriptions/SEI_and_SEIS_implementations_si_reference.md | Plot description markdown (SEI_and_SEIS_implementations_si_reference) |
| docs/tutorials/notebooks/01_SI_nobirths_logistic_growth.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/02_SI_wbirths_logistic_growth.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/03_SIS_nobirths_logistic_growth.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/04_SIR_nobirths_outbreak_size.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/05_SIR_wbirths_age_distribution.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/06_SIR_wbirths_natural_periodicity.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/07_SIR_CCS.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/08_2patch_SIR_wbirths_correlation.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/09_rabies_diffusion_1D.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/10_England_and_Wales.ipynb | Inserts plot-reading markdown cells after plot code cells (plus minor text normalization) |
| docs/tutorials/notebooks/SEI_and_SEIS_implementations.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/births.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/constant_pop.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/distributions.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/grid_examples.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/mortality.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/ri_exploration.ipynb | Inserts plot-reading markdown cells after plot code cells |
| docs/tutorials/notebooks/seasonality.ipynb | Inserts plot-reading markdown cells after plot code cells |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {"after_cell_containing": "import matplotlib.pyplot as plt", "md_file": "mortality_cdr_single_run.md"}, | ||
| {"after_cell_containing": "Observed CDRs per Run", "md_file": "mortality_cdr_sweep.md"}, | ||
| {"after_cell_containing": "plt.show()\n", "md_file": "mortality_estimator_single_run.md"} | ||
| ] |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Addresses Copilot review feedback that "plt.show()\n" is a fragile marker because it relies on a trailing newline to disambiguate two byte-for-byte identical plotting cells (mortality.ipynb cells 7 and 20 — the SEIR channels plot is produced from verbatim-identical code under both MortalityByCDR and MortalityByEstimator). No substring of one cell's source distinguishes it from the other, so a substring marker fundamentally cannot be both unique and stable for this pair. The fix adds an alternative target form: a manifest insert may now specify `after_cell_id` (the cell's stable Jupyter id field) instead of `after_cell_containing`. Cell IDs survive content edits and disambiguate duplicate-source cells reliably. Each insert must specify exactly one of the two forms; the script raises ValueError otherwise. Substring markers remain the default for the 65 other inserts in the manifest because they're more reviewer-friendly — a reviewer can read "Observed CDRs per Run" and understand the target without opening the notebook. Cell-ID markers are reserved for the rare case where substring matching is fundamentally insufficient. Mortality manifest entries updated: cell 7 (MortalityByCDR SEIR plot) -> after_cell_id 787332f1 cell 14 (MortalityByCDR sweep) -> unchanged (marker unique) cell 20 (MortalityByEstimator SEIR plot) -> after_cell_id 8f77666c Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previously --check would still write changes if any insert needed to be applied; only its exit code signaled the diff. Now --check passes write=False through to apply_to_notebook so CI invocations report what would change without touching the working tree. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
JSchripsema-IDM
left a comment
There was a problem hiding this comment.
Overall, I think adding descriptions of the plots is a good move, but the text seems overly verbose and doesn't always exactly match the text in the plots themselves. Separating notebooks aimed at showcasing model validity and trustworthiness and notebooks aimed at teaching new users how to configure and interpret simulations could also help provide some structure to improve usage of the RAG context window.
Claude also flagged this maintainability concern:
70 standalone .md files indexed by config.json create a silent drift risk: if plot code changes, descriptions won't flag as stale. The --check mode addresses insertion idempotency but not description currency. Recommend adding a brief note to tools/plot_descriptions/README.md about when descriptions need manual review (e.g., when the generating cell changes).
| "output_type": "stream", | ||
| "text": [ | ||
| "1,000,000 agents in 1 node(s): 100%|██████████| 730/730 [00:00<00:00, 1835.57it/s]\n" | ||
| "1,000,000 agents in 1 node(s): 100%|\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588\u2588| 730/730 [00:00<00:00, 1835.57it/s]\n" |
There was a problem hiding this comment.
Generally, we should avoid committing output cells to the repo because it can consume a lot of space and the notebooks are being executed as part of the doc build anyway.
| "- **Orange dashed \u2014 analytic logistic curve** with the known $\\beta = 0.05$ and $t_0 = 0$.\n", | ||
| "- **Black dashed \u2014 analytic logistic curve** with the same known $\\beta$ but a best-fit time offset of $t_0 \\approx -14$ days.\n", | ||
| "\n", | ||
| "The orange curve sits visibly to the left of the model in the exponential-growth region \u2014 i.e. the textbook logistic with no offset predicts the outbreak takes off about two weeks sooner than this realization actually did. The black curve, which only adjusts $t_0$, lines up almost perfectly with the blue model output across the entire trajectory. **The takeaway: stochastic delays among the very first infections shift the entire downstream trajectory in time without distorting its shape.** Once the early lag is absorbed into a single offset parameter, the deterministic logistic equation with the known $\\beta$ and population reproduces the discrete-time stochastic model \u2014 motivating the next section's strategy of fitting $\\beta$ jointly with $t_0$ rather than against the wall-clock day count.\n" |
There was a problem hiding this comment.
Make sure to incorporate recent updates to the mkdocs template in idm_standards--some custom JavaScript fixes the issue where inline LaTeX like that beta character wasn't rendered correctly.
There was a problem hiding this comment.
This is the file that needs to be updated: https://github.com/InstituteforDiseaseModeling/idm_standards/blob/main/docs_templates/mkdocs_template/docs/customization/javascript/mathjax.js
| "metadata": {}, | ||
| "source": [ | ||
| "A note that order of the model components matters, because of how recording of relevant statistics into outputs occurs. If transmission comes before susceptibility, then we have N=S+I+ΔI, because we record I, record ΔI, do transmission, then record S after the transmission process occurs. With susceptibility first, we record S, then record I, then do transmission (and record ΔI), and so we have N=S+I." | ||
| "A note that order of the model components matters, because of how recording of relevant statistics into outputs occurs. If transmission comes before susceptibility, then we have N=S+I+\u0394I, because we record I, record \u0394I, do transmission, then record S after the transmission process occurs. With susceptibility first, we record S, then record I, then do transmission (and record \u0394I), and so we have N=S+I." |
There was a problem hiding this comment.
I don't agree with the decision to remove characters/LaTeX in favor of Unicode since this will make the source a lot harder to read.
| "source": [ | ||
| "### Reading the \u03b2-recovery plots\n", | ||
| "\n", | ||
| "The cell emits two scatter plots side by side from a 10-seed sweep over $\\beta \\in \\{0.02, 0.03, \\ldots, 0.11\\}$:\n", |
There was a problem hiding this comment.
In the built docs, the plots are on top of each other, not side by side.
| "\n", | ||
| "Two nearly-overlapping curves on a shared log y-axis (1 to $10^6$) versus time in days (0 to ~730):\n", | ||
| "\n", | ||
| "- **Blue solid (thick) \u2014 currently infected** $I_t$ from `model.nodes.I`.\n", |
There was a problem hiding this comment.
These definitions don't match with the plot label, which is "population minus currently infected" and "susceptible"
| "\n", | ||
| "The cell emits four scatter plots from a 10-seed sweep over $\\beta \\in \\{0.010, 0.015, \\ldots, 0.055\\}$ paired with randomly drawn crude birth rates $\\mathrm{CBR} \\in [15, 50)$:\n", | ||
| "\n", | ||
| "- **Plot 1 \u2014 Fitted \u03b2 vs True \u03b2.** Axes 0.00\u20130.06 on both sides. Ten points cluster tightly along the identity line. Notably, two points near $\\beta = 0.040$ sit visibly *above* the diagonal (fitted \u2248 0.047) \u2014 an over-shoot rather than the systematic under-shoot seen in the births-free notebook.\n", |
There was a problem hiding this comment.
I know this isn't the focus of this PR, but adding labels to the plots themselves would help a lot with interpretation and understanding.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "Just an interesting note - because I kicked the simulations off with 3 infections rather than 1 (to ensure that the initial infection doesn't die before infecting someone), we consistently get a negative fit value for t0, and one that is a larger negative value for the lowest \u03b2 values." |
There was a problem hiding this comment.
It seems like you could delete this paragraph since it repeats the info in the one above and feels out of place following it.
| "source": [ | ||
| "### Reading the sanity-check plots\n", | ||
| "\n", | ||
| "Two side-by-side panels on log y-axes, both running from time 0 to ~3000 ticks:\n", |
There was a problem hiding this comment.
The labels on the plots themselves are very, very tiny!
| "\n", | ||
| "The first cell emits two figures from a single 30-year simulation of an SIR model with births and deaths at a crude birth rate of 90 per 1000:\n", | ||
| "\n", | ||
| "- **Top \u2014 compartment trajectories** versus tick (0 to ~11000 days). $S$ (blue) crashes from ~300000 down through a deep oscillation, $I$ (red) spikes briefly, $R$ (green) shoots up to ~275000, and $N$ (black) plateaus near 290000. After damped oscillations across the first ~2000 days, all four lines settle to a flat endemic equilibrium: $S^* \\approx 25000$, $I^* \\approx 3000$, $R^* \\approx 258000$.\n", |
There was a problem hiding this comment.
Both of these plots could use labels on the axes.
| "- **Y-axis \u2014 Average_Iage_observed**, the mean of an exponential fit to simulated ages at infection (after day $365 \\times 40$), spanning the same range.\n", | ||
| "- **Blue dots** \u2014 one per simulation; **red dashed** \u2014 the identity line.\n", | ||
| "\n", | ||
| "All 25 points sit tightly on the identity line across the full 0.73-2.33 year range. The notebook layout reserves space for two additional panels (susceptibility-age and population-age expected-vs-observed) which are commented out and therefore appear empty below the populated top panel. The accompanying printout shows the average fractional deviation, max deviation, and counts of runs exceeding 5% and 10% error \u2014 the verbal pass/fail criteria for the test.\n", |
There was a problem hiding this comment.
We should probably comment out this text and the empty plots if the plotting code is commented out.
Empirical impact — this PR stacks cleanly on top of #212 (common-gotchas)Ran a 21-prompt laser-generic test suite (from laser-mcp PR #33) against three corpus variants to isolate the marginal contribution of this PR:
This PR adds +2 to a1 count on top of #212, with the improvements concentrated exactly where the QED-in-figures framing predicts they should be — prompts about dynamics visualized in the tutorial figures. Per-prompt moves that this PR causes (row 3 vs row 2)
The improvements — p07 (mortality plots), p10 (balanced-pop plots), p17 & p19 (gravity-mixing spatial layouts) — are all prompts where the tutorial's figure is the load-bearing evidence for the modeling claim. That's exactly the case the "Reading the X plot" prose was designed to address: give text-only readers (and the LLM) the QED that only lands when you can see the figure. The regressions look variance-shaped:
Bigger pictureNeither this PR nor #212 breaks what the other fixes:
They compose. Recommend landing both. Repro detailsBuilt the row-2 / row-3 artifacts locally (GHA uv venv /tmp/lg-docs-venv --python python3.12
uv pip install -e . -r docs/requirements.txt 'numba>=0.60' --python /tmp/lg-docs-venv/bin/python --index-url https://pypi.org/simple
PYTHON=/tmp/lg-docs-venv/bin/python make docs-jenner ALLOW_NB_ERRORS=1Two small ancillary issues surfaced that would be worth follow-ups (not blocking this PR):
🤖 Analysis by Claude Code with @jonathanhhb |
Two surgical trims in response to JSchripsema-IDM review comments on PR #204: - 02_SI_wbirths_logistic_growth: the case-trace-and-fit description's closing clause about consistently-negative t0 repeated the following pre-existing "Just an interesting note" paragraph. Trimmed. - 05_SIR_wbirths_age_distribution: the Iage expected-vs-observed description's middle paragraph acknowledged empty subplot panels as intentional layout — extraneous meta-commentary. Trimmed to describe only the populated top panel. Also mirrored the trims to the source .md files under tools/plot_descriptions/ so `add_plot_descriptions.py --check` remains idempotent.
Per JSchripsema-IDM review: describe each curve by quoting its exact legend label in bold, followed by plain-text prose explanation. Applies to three descriptions Jen flagged: - 02_SI_wbirths sanity-check plot: "Population minus currently infected" and "Susceptible" labels (both inherited from the no-births notebook — the code actually plots I_t and N_t - S_t, which under ConstantPopVitalDynamics both equal I_t). - 02_SI_wbirths logistic-fit plot: "Model output" / "Logistic growth with known inputs, t0=0" / "Logistic growth with known inputs, best-fit t0 = 5.2". - 03_SIS_nobirths logistic-fit plot: same three-curve legend pattern. Same edit mirrored to the source .md files under tools/plot_descriptions/ so add_plot_descriptions.py --check remains idempotent. Kept the takeaway paragraphs, only rewrote the bullet section.
…eview) Per JSchripsema-IDM review pattern established in the prior commit: quote each curve's exact legend/label string in bold with double-quotes, followed by plain-text prose explanation. Applies to 19 descriptions across 12 notebooks: 01_SI_nobirths: sanity-check, logistic-fit-with-offset 02_SI_wbirths: row0 case-trace-and-fit 04_SIR_nobirths: SIR-compartment (5-curve legend: "S", "I", "R", "recoveries", "Est. R(∞)") 05_SIR_wbirths_age_distribution: SIR-trajectory + age histogram, age-at-infection fit, fraction-susceptible-by-age, population-age 08_2patch: coupling-vs-correlation (only "Fitted curve: ..." labeled; blue dots unlabeled — noted explicitly) 10_England_and_Wales: CCS calibration grid, top-10 wavelet-phase panels SEI_and_SEIS: SEI vs SEIS, SEI vs SI (both use compartment-comparison labels like "SEI Mean Susceptible") births: constant-CBR + time-varying-CBR (both labels) mortality: MortalityByCDR + MortalityByEstimator single-run channels ri_exploration: RI dose-timing histogram (mean line + normal PDF) seasonality: SI baseline All edits mirrored to source .md files under tools/plot_descriptions/; add_plot_descriptions.py --check remains idempotent (all 18 notebooks ok).
Final batch of the systematic label-matching pattern. Applies to
descriptions where the plot has legend/colorbar/label content that
didn't match the bullet phrasing:
07_SIR_CCS:
- patch_heatmap — colorbar labeled "Cases" (encodes log10(I/N))
- fit_surface_3d — 3D scatter labeled "Real Values"
- alpha_loglog_fit — "Simulations" + four "Fit R0=..." lines
09_rabies_diffusion_1D:
- wave_heatmap — colorbar labeled "Infectious"
- wavespeed_scatter — "Velocity left" / "Velocity right" / "y=x"
- relative_error_panels — "Right"/"Left" marker labels across 3 panels
10_England_and_Wales:
- best_combined — quotes "Observed", "Simulated", "Obs fit", "Sim fit"
across both stacked panels
constant_pop:
- seir_dynamics — 7 curves on dual y-axis, all quoted: "Total Population",
"Susceptible (S)", "Recovered (R)", "Exposed (E)", "Infectious (I)",
"Births", "Deaths"
seasonality:
- baseline_no_forcing — quotes 4 legend entries including
"Seasonality (scaled)" reference line
All idempotent: add_plot_descriptions.py --check passes across the 18
tutorial notebooks.
This closes out the review-comment cluster on exact-label-matching.
Files intentionally NOT touched: descriptions where the plot has no
per-curve legend (heatmaps with axis-only descriptions, per-sim "Sim {i+1}"
labels in SEI/SEIS suites, prose descriptions of single-series scatter)
— quoting nonexistent labels there would be worse than the current text.
Addresses the 4 plot-source concerns Jen raised. All are notebook code changes to the plotting cells themselves (not description edits) — the descriptions were already accurate; the plots weren't matching them. 01_SI_nobirths_logistic_growth (cell 13, β-recovery): - Two plt.figure() → subplots(1, 2, figsize=(14, 5)) so panels render side-by-side in the mkdocs-jupyter build (Jen: "In the built docs, the plots are on top of each other, not side by side"). Description already said "side by side"; now the plot actually does. 02_SI_wbirths_logistic_growth (cell 11, β- and CBR-recovery): - Four plt.figure() → subplots(2, 2, figsize=(12, 10)); added set_xlabel/set_ylabel/set_title to all four subplots (Jen: "adding labels to the plots themselves would help a lot"). Description's Plot 1/2/3/4 ordering maps to the 2×2 grid in reading order. 03_SIS_nobirths_logistic_growth (cell 7, sanity-check plots): - Bumped title fontsize to 14, axis-label fontsize to 12, tick-label size to 11 (Jen: "The labels on the plots themselves are very, very tiny!" — figsize=(18,5) rendered default 10pt text as visually tiny). - Reduced figsize (18, 5) → (14, 5) so labels aren't dwarfed by axes. - Corrected panel-2 title from "Population minus cumulative infections" to "Currently infected over time" and ylabel from "Incidence" to "Infectious" — the code plots model.nodes.I directly, so the old labels were semantically wrong. Description already said "Currently infected over time" for the right panel. 05_SIR_wbirths_age_distribution (cell 6, SIR trajectory + age histogram): - Added xlabel/ylabel/title to both figures (Jen: "Both of these plots could use labels on the axes"). - SIR trajectory: "Time (days)" / "Number of individuals" / "SIR compartment trajectories". - Age histogram: "Age at infection (days)" / "Count" / "Age at infection (after year 28)". Also updated two config.json markers whose old strings were removed by the plot rewrites: use "Relative error in fitted CBR" (was plt.ylim literal) and "Currently infected over time" (was old panel-2 title). Idempotency: add_plot_descriptions.py --check passes across all 18 tutorial notebooks. Python syntax checked on all 4 rewritten code cells.
|
I'm running a fresh jenner-generic-mcp test suite from Ye's branch against the Jenner doc built from the tip of this branch. |
The existing _TQDM_PROGRESS_RE was narrow — it only matched the
model.run() format ('N agents in M node(s): X%|...'). Notebooks that
use custom tqdm descriptions ('10/10 simulations: 45%|...') or no
description ('100%|... 25/25 [02:15<00:00, 5.41s/it]') slipped through.
Measured against the current #204-derived doc: 20,047 tqdm lines
total, of which only 38 matched the old regex (the model.run()
variant); 20,009 leaked into the vectorstore.
That leakage was the primary driver of the doc-size regression that
inflated the built doc from ~67k lines (baseline) to ~98k lines and
degraded RAG retrieval on the alpha test suite.
The new regex matches the tqdm rate marker at end-of-line ('it/s]' or
its inverse 's/it]'), a tqdm-specific signature that rarely appears
elsewhere. Catches all tqdm variants uniformly. Surrounding
classification logic (_collapse_progress_runs, the 'all lines match'
check on line 225) is unchanged.
Flagging a major discovery about how these descriptions interact with RAGRan a controlled A/B on the alpha-suite (20 prompt tests) using this branch, isolating the effect of the
The plot descriptions cause a 10× drop in first-try code-generation quality when they're in the RAG corpus. The total pass rate looks fine (19/21 vs baseline's 20/21) only because the runner retries up to 3 times with error feedback. Take away the retry loop and this is a hard regression. WhyPlot descriptions are prose about model behavior — "the orange sampled curve tracks the green input curve…", "peak infectious ≈ 16544, final recovered = 82391", etc. — using the same domain vocabulary as prompts asking for code. So when a user prompt like "Build a single-patch SI model with 50k agents…" hits the RAG, embeddings retrieve plot-description chunks alongside code chunks. The LLM sees prose about SI-model behavior instead of Confirmed by targeted A/BSame doc, same everything else — only difference is whether This is not a reason to block the plot descriptionsThey have genuine value for humans reading the notebooks and probably for image-adjacent LLM tasks (e.g. an LLM asked to interpret a specific plot). The damage is only when they land in the code-generation RAG corpus. Prerequisite for mergeBefore this can land, That'd be a small companion PR to laser-generic, small enough to bundle with this one if that's preferred. Happy to draft the stripping PR — flagging this here first because it's a class of concern that could affect other future prose additions to the notebooks (docstring bloat, worked-example prose, etc.), not just plot descriptions. Worth a broader conversation about "what belongs in the RAG-facing doc" as a design principle. |
…scriptions # Conflicts: # docs/tutorials/notebooks/06_SIR_wbirths_natural_periodicity.ipynb
…ct (#241) Adds a new `Execute Notebooks` workflow that owns notebook execution as a first-class CI stage, and reworks `Build Combined Doc` to consume the resulting artifact instead of re-executing notebooks itself. Also adds a `docs-jenner-artifact` Makefile target for the CI-side build path. ## Problems solved 1. Notebook execution used to fire only on releases (issue #186). Now fires on every main-push touching notebook or execution-affecting source, plus on manual dispatch. Broken notebooks are surfaced within minutes of landing. 2. Committed executed-notebook outputs no longer need to be kept fresh by convention. The CI artifact is the source of truth for downstream doc/RAG builds; the committed `outputs` field is decorative (kept only so github.com renders charts inline). 3. Doc builds no longer re-execute notebooks. `Build Combined Doc` downloads the executed_nbs artifact and just runs `make docs-jenner-artifact` (~2-3 min wall-clock vs the previous ~25 min). ## Architecture ``` Execute Notebooks trigger: push to main (paths-filtered) | workflow_dispatch cache: source-hash-keyed, invalidates on any input that can affect outputs (notebooks, src/**, pyproject.toml, docs/requirements.txt, docs/**/*.py, Makefile, the workflow file itself) gate: python docs/check_executed_nbs.py (fail on any nb error) manifest: writes dist/executed_nbs/manifest.json with commit_sha, source_hash, run_id, python_version, event_name, was_cache_hit, was_forced, was_allow_errors uploads: executed_nbs (400d) push-to-main OR clean dispatch executed_nbs-debug-<run_id> (30d) dispatch with allow_notebook_errors=1 executed_nbs-failed-<run_id> (30d) any failure path Build Combined Doc trigger: workflow_run completion of Execute Notebooks (auto-chain, restricted to push-triggered upstream runs so debug dispatches don't push a corpus PR to laser-mcp) | workflow_dispatch download: executed_nbs artifact (by run-id for workflow_run, latest successful for workflow_dispatch) compat gate (workflow_dispatch only): hashFiles of checkout vs manifest.source_hash; fail on mismatch unless use_latest_anyway=true build: make docs-jenner-artifact -> build site + concat sync: create/update laser-mcp PR ``` The two `hashFiles` lists (cache-key in execute-notebooks + compat-check in build-combined-doc) are literal duplicates by design — 7 patterns each, same order. Diverging silently breaks the compat gate. ## Deliberate non-goals - **No `pull_request` trigger.** Executing every notebook adds ~25 min per PR iteration; that cost is not affordable during review cycles. Broken notebooks land on main and are caught by the post-merge push run within minutes. Local `make docs-jenner-execute` or a manual `workflow_dispatch` remain as pre-merge validation options for PRs that specifically need them. - **No enforcement of source-only committed notebooks.** Contributors may commit executed OR stripped notebooks — both work identically for the doc build. Documented inline in the workflow file. If future consensus wants strict source-only commits (Option B), the enforcement point is a single check step in `github-actions.yml`. ## Bootstrap note On first enable, `workflow_dispatch` on Build Combined Doc will fail cleanly (no prior successful Execute Notebooks artifact to consume — `if_no_artifact_found: fail` catches this). Kick off Execute Notebooks once manually to seed. After that, either trigger works. ## Third-party action `dawidd6/action-download-artifact@v3` — needed because `actions/download-artifact` only fetches from the current run or a known run-id, and `workflow_dispatch` on Build Combined Doc has no upstream run-id to point at. This third-party action searches for "latest successful run of workflow X" — exactly the semantics we need. Widely used (~4M weekly downloads). ## Compatibility with other in-flight PRs Independent of #204 (plot descriptions), #240 (figtext form of #204), and #236 (concat improvements) — those change notebook / concat content. Compatible with #237 (skip re-exec in local `docs-jenner`) — together the two produce a clean Makefile trio: - `make docs-jenner` local fast path (per #237) - `make docs-jenner-execute` local full pipeline (per #237) - `make docs-jenner-artifact` CI path, EXEC_DIR pre-populated (this PR) Closes #186. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@JSchripsema-IDM Continuing this work in #240. Closing this. |
Refactors the 70 plot-description cells added by #204 from standalone markdown cells (### Reading the ... plot, with observation + interpretation + takeaway paragraphs) to plt.figtext() captions embedded inside the plot-generating code cells. Content is compressed to the takeaway sentence. Motivation: alpha-suite evaluation (jenner-generic-mcp, N=3 per arm, same generation quality: arm mean/20 a1 Δ vs no-descriptions no descriptions (control) 17.00 9.00 — fat markdown (#204 as-authored) 14.33 7.67 -2.67 slim markdown (H3 + takeaway) 16.33 8.33 -0.67 figtext (this PR) 17.00 9.00 +0.00 Standalone markdown description cells create their own chunks in the RAG corpus that compete for top-k retrieval on prompts like "build this SIR model" — the descriptions are semantically close ("simulate + plot") but have no API-usage content, so retrieval picks them up and displaces the chunks that actually show the model-building calls. plt.figtext embeds the description text inside the code cell's Python source. The concat pipeline emits that cell as a fenced Python block, and the ingest H4 splitter (laser-mcp #37) plus the recursive character splitter's fence-first separator keep the string fused with the surrounding plotting code. Result: description content is still in the RAG corpus, but as part of code chunks — no separate description-only chunks to compete with API-usage retrieval. Chunk-count evidence (laser-mcp ingest.py, chunk_size=1200): arm sections chunks Δ chunks vs no-descriptions no desc 515 972 — figtext 515 991 +19 (co-located, no new boundaries) slim 585 1037 +65 (new md sections) composite 585 1101 +129 (new md sections + fat content) The figtext arm adds essentially no new chunk boundaries. Takeaway extraction: for each description .md file, extracts the last bold span (>20 chars) containing a takeaway trigger word ("demonstrates", "takeaway", "confirms", ...). 68/70 auto-extract cleanly; 1 spot-fixed via MANUAL_TAKEAWAY, 1 confirmed OK via fallback path. Full logic lives in tools/apply_figtext_captions.py so the same manifest at tools/plot_descriptions/config.json remains the source of truth. Rendering: plt.figtext(0.5, -0.05, ..., ha="center", va="top", wrap=True, fontsize=8) places the caption just below the axes. Notebook outputs are NOT re-executed by this PR (docs-jenner uses committed outputs per #237), so the committed figure PNGs are unchanged; when notebooks are next re-executed for real (e.g. a release-time full build via docs-jenner-execute) the caption will render in the figure. Base branch: docs/notebook-plot-descriptions (#204's branch). This is a stacked PR — #204's markdown-form remains the base for reviewers to see what's being converted. Merge order intended: 1. #204 (as-authored, or force-updated to figtext form if reviewers prefer) 2. This PR (if not folded into #204) Related: - #204 base — the plot descriptions themselves - #236 — concat heading demote + oversize warning (independent) - #237 — docs-jenner skip re-execution (independent) - laser-mcp #37 — H4 in MarkdownHeaderTextSplitter (works with either form) - Alpha-suite runs under tests/prompt_test_suite_generic/output/experiment_204eval/ in laser-mcp; N=3 per arm, gpt-5-mini generation, gpt-4o review. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…m notebooks (#240) Two bundled improvements to the tutorial notebook corpus, both aimed at making it cleaner and more RAG-friendly. ## Plot descriptions in plt.figtext form (originally #240) The 70 plot descriptions authored in #204 (now closed) are converted from standalone `### Reading the ...` markdown cells into `plt.figtext(...)` captions inside the plot-generating code cells. Content compressed to the takeaway sentence per description. Why: the markdown form created its own RAG chunks that competed for retrieval on prompts like "build this SIR model" — semantically close ("simulate + plot") but no API content, so retrieval picked them up and displaced model-building code. plt.figtext embeds the description inside the code cell's Python source; the concat pipeline emits that cell as a fenced python block, and the ingest H4 splitter (laser-mcp #37) + fence-first character splitter keep the string fused with the surrounding plotting code. Alpha suite evidence (jenner-generic-mcp, N=3 per arm): Arm Mean/20 Δ vs no-descriptions No descriptions (baseline) 17.00 — Fat markdown (#204 as-authored) 14.33 -2.67 Slim markdown (H3 + takeaway) 16.33 -0.67 Figtext (this PR) 17.00 +0.00 Re-tested against latest main (post-#236 concat improvements, post-laser-mcp #37 H4 splitter): got 17, 18, 20 -> mean 18.33 on the fresh corpus, with run 3 hitting a clean-sweep 20/20. RAG-quality improvements compound cleanly. Copilot review flagged a rendering bug in seasonality.ipynb where the apply_figtext_captions tool injected figtext inside the plot_model / plot_si_model helper defs (baking one caption into every call) and after helper calls (attaching to empty figures because the helper had already called plt.show internally). Fixed by giving each helper a caption=None parameter, gating plt.figtext before plt.show inside; each call site passes its own caption via kwarg. ## tqdm progress-bar noise cleanup (new in this PR) Tutorial notebooks call model.run() with a tqdm progress display; nbconvert captures those mid-run snapshots ("N,NNN agents in M node(s): 37%|... it/s]") into cell outputs. That noise was leaking into three places: 1. Committed notebooks in docs/tutorials/notebooks/ carried 381 tqdm lines across 14 of 19 files. 2. CI executed_nbs artifact captured whatever nbconvert saw. MkDocs Deploy and Build Combined Doc both consume this artifact, so both were displaying / ingesting noise. 3. concat_mkdocs.py has a tqdm filter (from #229) but only fixed the RAG corpus, not the docs site or the artifact. New docs/tqdm_strip.py owns the shared regex + a small library + CLI. Wired into docs/execute_notebooks.py, so future CI artifacts ship clean by construction — docs site (via mkdocs-ghp overlay) and RAG corpus (via Build Combined Doc concat) both benefit without further changes. concat_mkdocs.py imports the shared regex; keeps its own filter as a defensive second pass. Applied strip to all 19 committed notebooks: 381 lines removed across 14 files. New pytest gate tests/test_notebooks_no_tqdm_crud.py fails the existing github-actions.yml check matrix entry if any committed notebook re-acquires tqdm output. No new workflow. ## Files changed - tools/apply_figtext_captions.py (new) - figtext injector tool - tools/plot_descriptions/ (new) - 70 description .md files + config.json - docs/tqdm_strip.py (new) - shared regex + strip CLI - docs/execute_notebooks.py - post-execute strip step - docs/concat_mkdocs.py - imports shared regex - tests/test_notebooks_no_tqdm_crud.py (new) - CI gate - 18 tutorial notebooks - figtext captions in plot code cells + tqdm crud stripped
The tutorial notebooks under docs/tutorials/notebooks/ end with matplotlib figures with no surrounding prose explaining what the figures show. When these notebooks are fed to a documentation LLM / RAG (e.g. laser-mcp), the model cannot read the rendered PNG outputs nearly as well as it can read a pre-existing verbal summary of the axes, curves, and takeaway. This commit adds that prose and the tooling to maintain it.
For each plot-bearing code cell, inserts one '### Reading the ... plot' markdown cell directly after the cell, nested under the existing '##' section heading. Each description follows an observation -> interpretation -> application sequence:
Notebooks covered (18 total, 70 plot cells):
01_SI_nobirths_logistic_growth 4 plots
02_SI_wbirths_logistic_growth 4 plots
03_SIS_nobirths_logistic_growth 4 plots
04_SIR_nobirths_outbreak_size 3 plots
05_SIR_wbirths_age_distribution 5 plots
06_SIR_wbirths_natural_periodicity 2 plots
07_SIR_CCS 4 plots
08_2patch_SIR_wbirths_correlation 1 plot
09_rabies_diffusion_1D 3 plots
10_England_and_Wales 4 plots
SEI_and_SEIS_implementations 5 plots
births 2 plots
constant_pop 1 plot
distributions 10 plots
grid_examples 6 plots
mortality 3 plots
ri_exploration 4 plots
seasonality 6 plots
Excluded: EW_analysis.ipynb (no rendered plots), demographics/age_pyramid and demographics/kmestimator (outputs cleared, distinct from the rendered copies already described in the laser repo).
Adds tools/add_plot_descriptions.py, a manifest-driven helper that applies these inserts idempotently. The manifest at tools/plot_descriptions/ config.json maps each notebook to a list of (after-cell marker, md-file) pairs; the marker is a unique substring of the plot code cell, more durable than a brittle cell index. The prose itself lives in standalone .md files alongside the manifest so PR diffs are reviewable as plain markdown rather than JSON-escaped strings.
The script supports --check for CI: exits non-zero if any notebook would change, catching cases where someone edits a notebook and accidentally drops a description cell. Marker matching is restricted to code cells so the script remains idempotent even when an inserted description quotes the marker string verbatim in its own prose.
No notebook code was modified; existing plot outputs are preserved.