Adding population parameters as attributes by sschildhauer · Pull Request #141 · ucdavis/serodynamics

sschildhauer · 2025-12-09T19:13:28Z

The primary goal of this pull request is to have a command that allows us to monitor population parameters and include them as attributes. Given that the population parameters are quite large, the default is currently set to not include them, which could make sense in the situation that you are modeling specifically for input into serocalculator. However, if comparing the estimated population level parameters between stratification it may make sense to include population level parameters.

as_case_data also had to be modified in order to pass tests as the current code was restructuring the output data in a way that was making tests fail -- the order of the columns would differ between runs. I would have preferred to fix this in another PR but I was not able to get reviews to check without making this alteration.

codecov · 2025-12-09T19:18:35Z

❌ 5 Tests Failed:

Tests completed	Failed	Passed	Skipped
29	5	24	0

View the top 3 failed test(s) by shortest run time

run_mod::results_are_consistent_with_unstratified_SEES_data_with_jags.post_included

Stack Traces | 0.022s run time

Error in `tibble(Iteration = NULL, Chain = NULL, value = NULL, Subject = NULL, Parameter = NULL, Iso_type = NULL, Stratification = NULL)`: could not find function "tibble"
Backtrace:
    x
 1. +-base::suppressWarnings(...) at test-run_mod.R:135:5
 2. | \-base::withCallingHandlers(...)
 3. \-serodynamics::run_mod(...)

run_mod::results_are_consistent_with_unstratified_SEES_data_with_modified_priors

Stack Traces | 0.023s run time

Error in `tibble(Iteration = NULL, Chain = NULL, value = NULL, Subject = NULL, Parameter = NULL, Iso_type = NULL, Stratification = NULL)`: could not find function "tibble"
Backtrace:
    x
 1. +-base::suppressWarnings(...) at test-run_mod.R:169:5
 2. | \-base::withCallingHandlers(...)
 3. \-serodynamics::run_mod(...)

run_mod::results_are_consistent_with_unstratified_SEES_data

Stack Traces | 0.035s run time

Error in `tibble(Iteration = NULL, Chain = NULL, value = NULL, Subject = NULL, Parameter = NULL, Iso_type = NULL, Stratification = NULL)`: could not find function "tibble"
Backtrace:
    x
 1. +-base::suppressWarnings(...) at test-run_mod.R:96:5
 2. | \-base::withCallingHandlers(...)
 3. \-serodynamics::run_mod(...)

run_mod::results_are_consistent_with_SEES_data

Stack Traces | 0.053s run time

Error in `tibble(Iteration = NULL, Chain = NULL, value = NULL, Subject = NULL, Parameter = NULL, Iso_type = NULL, Stratification = NULL)`: could not find function "tibble"
Backtrace:
    x
 1. +-base::suppressWarnings(...) at test-run_mod.R:57:5
 2. | \-base::withCallingHandlers(...)
 3. \-serodynamics::run_mod(...)

run_mod::results_are_consistent_with_simulated_data

Stack Traces | 0.286s run time

Error in `tibble(Iteration = NULL, Chain = NULL, value = NULL, Subject = NULL, Parameter = NULL, Iso_type = NULL, Stratification = NULL)`: could not find function "tibble"
Backtrace:
    x
 1. +-base::suppressWarnings(...) at test-run_mod.R:16:5
 2. | \-base::withCallingHandlers(...)
 3. \-serodynamics::run_mod(...)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

github-actions · 2025-12-09T19:39:53Z

📖 https://ucd-serg.github.io/serodynamics/preview/pr141
Preview documentation for this PR (at commit 8d91c90)

sschildhauer · 2025-12-10T05:16:28Z

I am unsure why these final tests aren't passing. @Kwan-Jenny Any thoughts?

…fferent population parameters

NEWS.md

pkgdown/_pkgdown.yml

Kwan-Jenny · 2025-12-16T15:58:28Z

I am unsure why these final tests aren't passing. @Kwan-Jenny Any thoughts?

I am not sure what’s causing these final test failures. It may be helpful to ask @d-morrison for his thoughts.

into mon_popparams

sschildhauer · 2025-12-20T19:30:12Z

Hi @d-morrison , I have not seen this review fail message. I am a but confused how this came up as I made very few changes from the last review. It looks like it might have something to do with package dependencies, but unsure. Once this is done I can send up the posterior predictive check for review.

Copilot

Pull request overview

Copilot reviewed 29 out of 34 changed files in this pull request and generated 3 comments.

tests/testthat/test-run_mod.R

Copilot · 2026-04-04T13:52:06Z

tests/testthat/test-run_mod.R

    results |>
      expect_snapshot_data(
        "sim-strat-curve-params",
        variant = darwin_variant()
      )


All expect_snapshot_data(..., variant = darwin_variant()) calls will use the default snapshot variant (no variant) on Linux/Windows because darwin_variant() returns NULL there. However, the PR removes the default snapshot directory (tests/testthat/_snaps/run_mod/), leaving only tests/testthat/_snaps/darwin/run_mod/, so these snapshot expectations will fail on non-macOS CI. Re-add/commit the non-variant snapshots under _snaps/run_mod/ (including the new popparam-summary-stats.csv) or adjust the variant logic so non-darwin platforms resolve to an existing snapshot set.

Copilot

Pull request overview

Copilot reviewed 29 out of 34 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

R/Run_Mod.R:39

The documented returned columns use lowercase iteration/chain, but the actual output columns are Iteration and Chain. Please update the roxygen list to match the real column names so users can reliably work with run_mod() output.

#' including the following:
#'   - `iteration` = Number of sampling iterations
#'   - `chain` = Number of MCMC chains run; between 1 and 4
#'   - `Parameter` = Parameter being estimated. Includes the following:

tests/testthat/test-run_mod.R

sschildhauer · 2026-04-04T14:22:29Z

@copilot apply changes based on the comments in this thread

…add Linux snapshots Agent-Logs-Url: https://github.com/ucdavis/serodynamics/sessions/4b06b305-9888-4e79-9089-e597844015fc Co-authored-by: sschildhauer <165851188+sschildhauer@users.noreply.github.com>

Copilot · 2026-04-04T14:31:04Z

@copilot apply changes based on the comments in this thread

Applied in commit 43129e0:

R/as_case_data.R: switched from union("case_data", data_class) to union("case_data", current_atts$class) so tibble classes (tbl_df, tbl) are always present regardless of input type, and the now-unused data_class <- class(data) line was removed
tests/testthat/test-run_mod.R: added dplyr::arrange(Parameter) before the popparam summary snapshot to ensure stable row ordering

@copilot apply changes based on the comments in this thread

Applied in commit 43129e0:

R/as_case_data.R: fixed class vector to use current_atts$class so tibble subclasses are always preserved
tests/testthat/test-run_mod.R: added expect_equal(names(attributes(results))[1:3], c("names", "row.names", "class")) after the expect_setequal() check to explicitly guard attribute ordering
Also generated Linux-platform CSV snapshots for all test-run_mod.R tests under _snaps/run_mod/

@copilot, show me how to fix the above open reviews on this pull request.

Fixed in commit 43129e0. The open issues were:

R/as_case_data.R was using input class (data_class) instead of post-processing class (current_atts$class), dropping tbl_df/tbl when input was a plain data.frame — fixed
test-run_mod.R needed a strict attribute ordering check (not just expect_setequal) — added expect_equal(names(attributes(results))[1:3], c("names", "row.names", "class"))
Population parameter summary needed dplyr::arrange(Parameter) for stable row order before snapshotting
Linux-platform CSV snapshots were missing — regenerated all under _snaps/run_mod/

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Merge branch 'mon_popparams' of https://github.com/UCD-SERG/serodynamics into mon_popparams # Conflicts: # R/as_case_data.R

Copilot

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

R/prep_popparams.R

R/ex_popparams.R

tests/testthat/test-run_mod.R

sschildhauer · 2026-04-08T06:07:03Z

@copilot Please suggest a solution about how to make sure all tests are passing in the reviews above. Only focus on making the tests pass without recommending unrelated changes. Please include recommendations in this thread.

Copilot

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

R/Run_Mod.R:267

calc_fit_mod() is called with original_data = dl_sub, but when strat has multiple levels dl_sub only contains the last stratum processed in the loop. This means fitted_residuals will be computed for only that final stratum and will silently omit residuals for earlier strata. Consider computing fitted/residuals per-stratum inside the loop and binding them, or pass the full input data (and/or include stratification in the join) so fitted_residuals covers all strata in jags_out.

  # Calculating fitted and residuals
  # Renaming columns using attributes from as_case_data
  fit_res <- calc_fit_mod(modeled_dat = jags_out,
                          original_data = dl_sub)
  jags_out <- jags_out |>
    structure(fitted_residuals = fit_res)

… at position [2] Agent-Logs-Url: https://github.com/ucdavis/serodynamics/sessions/93798c2d-4272-4802-b85a-3f031f17c05c Co-authored-by: sschildhauer <165851188+sschildhauer@users.noreply.github.com>

Copilot · 2026-04-08T06:45:57Z

@copilot Please suggest a solution about how to make sure all tests are passing in the reviews above. Only focus on making the tests pass without recommending unrelated changes. Please include recomme...

The only remaining failure was test-as_case_data.R:21 on all platforms. The root cause was that as_case_data.R had been reverted to use structure(), which in R always appends modified attributes at the end — so class ended up at position [6] instead of position [2] as the snapshot expected.

Fixed in commit 316f600: restored the explicit attribute list construction using an anonymous function (\(x) { ... })() that rebuilds attributes in the correct order (names, row.names, class, id_var, ...). All 5 tests in test-as_case_data.R and all 13 tests in test-run_mod.R pass locally now.

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Kwan-Jenny

Everything looks good and ready to move forward. It’s time to ask Ezra for a review. Great job, Sam!

sschildhauer · 2026-04-09T03:06:37Z

Hi @d-morrison, this PR developed change more files than I had originally thought. A lot of snaps and documentation were changed. Apologies for having so many changes in one PR. I am still working through better compartmentalizing.

Changes non-related to my primary PR:
There was a problem with the as_case_data tests. I had to specifically force the order of columns to pass reviews -- this might have been an artifact from before.

Reviews were not passing for some of the documentation, specifically when referring to tbl_df and referencing run_mod, so I had to change the documentation across many functions. I am not sure what changed here, if it was a package requirement that changed.

Thinking about it now, perhaps I should have started another branch where I fixed the above issues but got caught up in this PR that I was having a hard time sorting out what was related and what was not. Happy to discuss further.

Thank you so much for taking a look!

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

R/Run_Mod.R:264

calc_fit_mod() is called after the stratification loop with original_data = dl_sub, but dl_sub is the last stratum subset. For stratified runs (and especially when subject IDs repeat across strata), this can produce incorrect fitted/residuals (missing strata or duplicated merges) because fitted/residuals are computed against only one subset without stratification context. Consider computing fitted/residuals per-stratum inside the loop (and binding results, optionally adding a Stratification column), or otherwise ensure the full original data and stratification key are used when calculating fitted/residuals.

    structure(priors = attributes(priorspec)$used_priors)
  
  # Calculating fitted and residuals
  # Renaming columns using attributes from as_case_data
  fit_res <- calc_fit_mod(modeled_dat = jags_out,

sschildhauer added 5 commits December 9, 2025 10:55

Updating run_mod to monitor pop_params

1194641

Updating News

216de96

Updating run_mod for pop_params

3b44be0

Increment version number to 0.0.0.9044

d167acf

Updating run_mod tests to include population parameters

806389c

Adding visisble binding

12fd909

Updating lint for run_mod

9e5b61f

sschildhauer requested a review from Kwan-Jenny December 10, 2025 05:16

sschildhauer requested a review from d-morrison December 10, 2025 17:49

sschildhauer added 5 commits December 10, 2025 11:00

Comitting unpack_jags for unpacking an MCMC object with all of the di…

c1956ce

…fferent population parameters

jags_unpack documentation

c1ccc63

Updating run_mod and documentation to include prec.logy

23cb959

Adding unpack_jags to yaml

ab0876b

documentation and lint

9c6255d

d-morrison reviewed Dec 16, 2025

View reviewed changes

NEWS.md Outdated Show resolved Hide resolved

d-morrison reviewed Dec 16, 2025

View reviewed changes

pkgdown/_pkgdown.yml Outdated Show resolved Hide resolved

d-morrison added 2 commits December 16, 2025 01:13

Merge branch 'main' into mon_popparams

e819741

Merge branch 'main' into mon_popparams

808eb60

sschildhauer added 6 commits December 18, 2025 12:45

Increment version number to 0.0.0.9045

6f7b224

Updating tests for monitor popupation parameters

1ea38a0

Merge branch 'mon_popparams' of https://github.com/UCD-SERG/serodynamics

920ae66

into mon_popparams

Taking unpack_jags out of pkgdown

b78a454

Updating news

2d5a841

Updating the documentation.

6b63587

sschildhauer requested a review from d-morrison December 20, 2025 19:26

sschildhauer added 2 commits April 3, 2026 21:33

Adding a snap for fitted because it was declared

d282ba5

Fixing run_mod snaps

34ebde3

Copilot AI review requested due to automatic review settings April 4, 2026 13:45

Copilot started reviewing on behalf of sschildhauer April 4, 2026 13:45 View session

Copilot AI reviewed Apr 4, 2026

View reviewed changes

sschildhauer added 2 commits April 4, 2026 21:01

Error in commas for snaps. Running again

9b93578

Redoing snaps

36891b9

Copilot AI review requested due to automatic review settings April 4, 2026 14:10

Copilot started reviewing on behalf of sschildhauer April 4, 2026 14:11 View session

Copilot AI reviewed Apr 4, 2026

View reviewed changes

tests/testthat/test-run_mod.R Show resolved Hide resolved

Copilot started work on behalf of sschildhauer April 4, 2026 14:22 View session

fix: use current_atts$class in as_case_data, add attr ordering test, …

43129e0

…add Linux snapshots Agent-Logs-Url: https://github.com/ucdavis/serodynamics/sessions/4b06b305-9888-4e79-9089-e597844015fc Co-authored-by: sschildhauer <165851188+sschildhauer@users.noreply.github.com>

Copilot finished work on behalf of sschildhauer April 4, 2026 14:31

Copilot AI reviewed Apr 5, 2026

View reviewed changes

sschildhauer added 3 commits April 8, 2026 09:56

Reverting as_case_data to the old code

0cd7be6

pulling from remote

a23e7a4

Merge branch 'mon_popparams' of https://github.com/UCD-SERG/serodynamics into mon_popparams # Conflicts: # R/as_case_data.R

Updating as_case_data snap

a9dfb8f

Copilot AI reviewed Apr 8, 2026

View reviewed changes

R/prep_popparams.R Show resolved Hide resolved

R/ex_popparams.R Show resolved Hide resolved

tests/testthat/test-run_mod.R Show resolved Hide resolved

Copilot AI reviewed Apr 8, 2026

View reviewed changes

fix: restore explicit attribute ordering in as_case_data to put class…

316f600

… at position [2] Agent-Logs-Url: https://github.com/ucdavis/serodynamics/sessions/93798c2d-4272-4802-b85a-3f031f17c05c Co-authored-by: sschildhauer <165851188+sschildhauer@users.noreply.github.com>

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Kwan-Jenny reviewed Apr 8, 2026

View reviewed changes

sschildhauer added 2 commits April 9, 2026 19:55

Reverting as_case_data to old code

04b8a92

Fixing as_case_data again.

8d91c90

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Conversation

sschildhauer commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 5 Tests Failed:

Uh oh!

github-actions bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sschildhauer commented Dec 10, 2025

Uh oh!

Uh oh!

Uh oh!

Kwan-Jenny commented Dec 16, 2025

Uh oh!

sschildhauer commented Dec 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

sschildhauer commented Apr 4, 2026

Uh oh!

Copilot AI commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sschildhauer commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Kwan-Jenny left a comment

Choose a reason for hiding this comment

Uh oh!

sschildhauer commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sschildhauer commented Dec 9, 2025 •

edited

Loading

codecov bot commented Dec 9, 2025 •

edited

Loading

github-actions bot commented Dec 9, 2025 •

edited

Loading

Copilot AI commented Apr 4, 2026 •

edited

Loading

sschildhauer commented Apr 8, 2026 •

edited

Loading