Conversation
…inal into Understanding LMs, group goodness of fit/diagnostics/model selection under Assessing model fit Agent-Logs-Url: https://github.com/d-morrison/rme/sessions/2640c9c0-d0e3-4c08-a9e1-506e9f9ea53f Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
|
@copilot resolve merge conflicts |
…cture from main Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
Resolved in commit 2ebae7c. The main branch had split
|
…e _sec_linreg_assess.qmd for assessing model fit subsections Agent-Logs-Url: https://github.com/d-morrison/rme/sessions/04f5a981-b328-4b54-b877-dd0c5e665850 Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
…ction.qmd so it's defined before its first use Agent-Logs-Url: https://github.com/d-morrison/rme/sessions/87923ea1-e789-445b-a0dc-37d8bb3247fe Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
Fixed in commit d62a233. The CI failure was caused by |
Built to branch |
There was a problem hiding this comment.
Pull request overview
Reorganizes the linear regression Quarto content to match the new subfile structure from main, grouping related material under more coherent parent sections and fixing an ordering issue with a shared plotting helper.
Changes:
- Moves categorical/ordinal covariate material under “Understanding Gaussian Linear Regression Models”.
- Creates a new “Assessing model fit” parent section to group goodness-of-fit, diagnostics, and model selection.
- Relocates
plot_PIs_and_CIsinto the prediction section so it’s defined before later uses.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Linear-models-overview.qmd | Replaces several top-level sections with a single “Assessing model fit” include to reflect the new structure. |
| _sec_linreg_understanding.qmd | Adds includes for categorical and ordinal covariate subfiles under the “Understanding” section. |
| _sec_linreg_prediction.qmd | Adds the plot_PIs_and_CIs helper earlier in the document to resolve ordering. |
| _sec_linreg_assess.qmd | New subfile that groups GOF, diagnostics, and model selection under one parent section. |
| _sec_linreg_gof.qmd | Demotes headings and removes the local plot_PIs_and_CIs definition after reorg. |
| _sec_linreg_diagnostics.qmd | Demotes headings to fit under the new “Assessing model fit” hierarchy. |
| _sec_linreg_model_selection.qmd | Demotes headings to fit under the new “Assessing model fit” hierarchy. |
| _sec_linreg_categorical.qmd | Demotes headings to fit under the new “Understanding” hierarchy. |
| _sec_linreg_ordinal.qmd | Demotes a callout header to fit under the new “Understanding” hierarchy. |
| plot_PIs_and_CIs <- function(model, data) { | ||
| cis <- model |> | ||
| predict(interval = "confidence") |> | ||
| suppressWarnings() |> | ||
| tibble::as_tibble() | ||
| names(cis) <- paste("ci", names(cis), sep = "_") | ||
|
|
||
| preds <- model |> | ||
| predict(interval = "predict") |> | ||
| suppressWarnings() |> | ||
| tibble::as_tibble() | ||
| names(preds) <- paste("pred", names(preds), sep = "_") | ||
| dplyr::bind_cols(bw, cis, preds) |> |
There was a problem hiding this comment.
plot_PIs_and_CIs() currently ignores its data argument and instead hard-codes bw in bind_cols(). This makes the function behave incorrectly for any dataset other than bw, and can also break if the model’s effective rows differ from bw (e.g., na.action), since predict() is called without newdata. Use the provided data argument consistently (including passing it as newdata to predict() and binding data, not bw), and consider allowing the function to work with the piped call style used later in the GOF section.
| plot_PIs_and_CIs <- function(model, data) { | |
| cis <- model |> | |
| predict(interval = "confidence") |> | |
| suppressWarnings() |> | |
| tibble::as_tibble() | |
| names(cis) <- paste("ci", names(cis), sep = "_") | |
| preds <- model |> | |
| predict(interval = "predict") |> | |
| suppressWarnings() |> | |
| tibble::as_tibble() | |
| names(preds) <- paste("pred", names(preds), sep = "_") | |
| dplyr::bind_cols(bw, cis, preds) |> | |
| plot_PIs_and_CIs <- function(data, model = NULL) { | |
| if (is.null(model)) { | |
| rlang::abort("`model` must be supplied to `plot_PIs_and_CIs()`.") | |
| } | |
| if (!inherits(data, "data.frame") && inherits(model, "data.frame")) { | |
| tmp <- data | |
| data <- model | |
| model <- tmp | |
| } | |
| cis <- model |> | |
| predict(newdata = data, interval = "confidence") |> | |
| suppressWarnings() |> | |
| tibble::as_tibble() | |
| names(cis) <- paste("ci", names(cis), sep = "_") | |
| preds <- model |> | |
| predict(newdata = data, interval = "predict") |> | |
| suppressWarnings() |> | |
| tibble::as_tibble() | |
| names(preds) <- paste("pred", names(preds), sep = "_") | |
| dplyr::bind_cols(data, cis, preds) |> |
| ### Null Deviance | ||
|
|
||
| Similarly, the *least* complicated model we could fit would have only one mean parameter, an intercept: | ||
|
|
There was a problem hiding this comment.
In this section, the later figure chunk (lm0 |> plot_PIs_and_CIs()) will fail if plot_PIs_and_CIs requires a data argument (as in its current definition). Update the call to pass the dataset explicitly (e.g., lm0 |> plot_PIs_and_CIs(bw)), or make the helper accept a default data value that works for the null-model plot.
| If there are more than two observed values, we can't just use a single variable with 0s and 1s. | ||
|
|
||
| ## | ||
| ### |
There was a problem hiding this comment.
There is an empty section header (### with no title). This will render as a blank heading and may add an empty entry to the table of contents / navigation. Replace it with a meaningful heading (or remove it if it’s only meant as spacing).
| ### | |
| ### Example: `iris` |
- Update plot_PIs_and_CIs() to use data argument consistently: - Change signature to (data, model = NULL) - Add null check for model - Add argument-swap guard for backward compatibility - Use newdata = data in predict() calls - Use data instead of hard-coded bw in bind_cols() - Fix lm0 |> plot_PIs_and_CIs() call to pass bw explicitly - Fix empty ### section header to ### Example: iris Agent-Logs-Url: https://github.com/d-morrison/rme/sessions/e7bcda97-e3b1-42c7-beff-b66ce7e22140 Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
…-431 Apply review comments from PR #431
Linear-models-overview.qmdto use{{< include >}}directives with the correct structure_sec_linreg_categorical.qmd,_sec_linreg_gof.qmd,_sec_linreg_diagnostics.qmd,_sec_linreg_model_selection.qmd,_sec_linreg_ordinal.qmdby one levelLinear-models-overview.qmdinto_sec_linreg_understanding.qmdLinear-models-overview.qmdinto new_sec_linreg_assess.qmdsubfileplot_PIs_and_CIswas defined in_sec_linreg_gof.qmdbut used in_sec_linreg_prediction.qmdwhich now comes before gof in the section order. Moved definition to_sec_linreg_prediction.qmd.