Add time-varying covariate sampling, and anonymization function - #12
Add time-varying covariate sampling, and anonymization function#12roninsightrx wants to merge 20 commits into
Conversation
roninsightrx
left a comment
roninsightrx
left a comment
There was a problem hiding this comment.
Code review — time-varying MICE sampling
Reviewed the diff and ran the real (unmocked) MICE transition path in R, since every test mocks both sample_covariates_mice and impute_mice_transition_step — so the actual mice path is never exercised end-to-end. Two crashes on realistic default-setting inputs that the mocked tests cannot catch (#1, #2 inline), plus efficiency/reuse cleanups.
Top priority: add at least one un-mocked end-to-end test through the real mice path.
Conventions clean: DESCRIPTION bumped 9010→9011, man/ regenerated and matches roxygen, NAMESPACE export added.
| time_varying_covs | ||
| ) { | ||
| lag_covs <- paste0(time_varying_covs, "_lag") | ||
|
|
There was a problem hiding this comment.
Crash (confirmed live): default measurement_pattern="change" + single-row subject + ≥2 time-varying covs.
sapply(profile[time_varying_covs], ...) over a 1-row profile returns a length-k vector with no dim. The is.null(dim(update)) branch reshapes it to a k×1 matrix, then colnames(update) <- time_varying_covs (line 481) assigns k names to 1 column → length of 'dimnames' [2] not equal to array extent.
Reproduced with a dataset mixing multi-row and single-row subjects and time_varying_covs=c("WT","SCR") — crashes with default settings. Force matrix shape explicitly (as the nonmissing/all branches do).
| previous_time, | ||
| time_var, | ||
| static_covs, | ||
| time_varying_covs, |
There was a problem hiding this comment.
Crash / silent corruption (confirmed live): internal column names collide with user covariates.
delta_time (here), Type (line 545), and <cov>_lag are introduced as working columns with no guard. A user covariate named delta_time crashes with Can't transform a data frame with duplicate names; Type / X_lag (alongside a real X) silently overwrite or zero out a legitimate predictor.
Namespace these with a . prefix, matching the existing .previous_time.
| current[active_subjects, ] <- active_current | ||
| replicate_rows[[time_idx]] <- active_current | ||
| } | ||
| } |
There was a problem hiding this comment.
Efficiency: full mice::mice() refit every timepoint.
impute_mice_transition_step() is called once per time_idx inside this loop, and each call refits mice from scratch on the entire transition_data. The transition model X(t) ~ X(t-1), time, delta_time, statics is identical at every step.
With n=1000 over 20 timepoints that's 19 full chained-equation fits over ~20k rows instead of one reused fit. Inside impute_mice_transition_step the factor re-coercion and make.predictorMatrix/make.method are also rebuilt every step though the structure never changes.
| method = method, | ||
| ... | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Reuse: duplicated mice-runner block.
This re-implements the exact block from sample_covariates_mice (R/sample_covariates_mice.R:86-118): bind Original/Simulated via a Type column, make.predictorMatrix + zero Type, make.method, mice(m=1, ...) under suppressWarnings, then complete(action="long") |> filter(Type=="Simulated") |> select().
Extract a shared helper so the two paths can't drift (e.g. the tidyr::complete(action=) usage, Type bookkeeping, warning handling).
| ... | ||
| ) | ||
|
|
||
| subject_profiles <- if (is.null(time_grid)) { |
There was a problem hiding this comment.
Altitude: ... forwarded to two destinations; conditional not a real parameter.
... goes to both sample_covariates_mice (baseline) and mice::mice() (every transition step). sample_covariates(method="mice_timevarying", conditional=...) sends conditional into both — it works today only because mice() silently swallows unknown args, and transitions end up unconstrained. The other sample_covariates_* methods expose conditional as a formal parameter; this one should too.
| make_mice_update_matrix <- function( | ||
| profile, | ||
| time_varying_covs, | ||
| measurement_pattern |
There was a problem hiding this comment.
Minor efficiency: O(n²) propensity matching loop.
available <- setdiff(available, candidates) rebuilds the candidate set and recomputes abs(score) over the full remaining pool each iteration. With ~1000 observed subjects that's ~500k comparisons. A logical used mask (used[candidates] <- TRUE; subset by !used) makes it linear.
Address review on PR #12: - Fix crash on default measurement_pattern="change" with single-row subjects and >=2 time-varying covariates: sapply() over a 1-row profile dropped the matrix dims; force explicit nrow/ncol so the colnames<- assignment no longer fails. - Namespace internal working columns (.delta_time, .Type, .<cov>_lag) with a "." prefix, matching .previous_time, so they cannot collide with or silently overwrite user covariates. - Extract shared run_mice_simulation() helper used by both sample_covariates_mice() and the transition sampler so the Type bookkeeping and predictor/method wiring cannot drift. - Expose conditional as a formal parameter forwarded only to the baseline sample, instead of leaking through ... into mice::mice(). - Replace the O(n^2) setdiff-based propensity matching loop with a logical "used" mask. - Add un-mocked end-to-end tests through the real mice path, plus regression tests for the single-row crash and internal-name collisions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough review — and especially for running the real mice path. Addressed in f2a75ba: #1 — crash on #2 — internal column name collisions. Fixed. #4 — duplicated mice-runner block. Extracted #5 — #6 — O(n²) propensity loop. Replaced the Top priority — un-mocked end-to-end test. Added one that runs the full real mice path (baseline + sequential transitions, categorical covariate, #3 — full |
Address review findings on PR #13: - apply_simulated_concentrations: match FeRx output to observation rows by ID and TIME instead of positional copy, which could silently scramble concentrations when FeRx reorders rows - apply_anonymize_lloq: guard against NA simulated DV (avoids all-NA rows on the drop path and a subscript error on the cens path) - sample_covariates_mice_timevarying: reject design_id_var that collides with id_var, time_var, or a covariate name - simulate_anonymized_concentrations: stop forcing a fixed seed of 42 when seed is NULL; draw independently instead - build_anonymized_simulation_input: warn and skip mis-aligned subjects rather than aborting the whole run; match designs on character-coerced IDs; use pre-split lookups instead of per-subject full-table scans - remove dead dictionary args and unused local bindings; use NA_character_ for the time_grid profile placeholder - add regression tests; bump version to 0.0.0.9013 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ize-nonmem-dataset
Add sample_covariates_lme_timevarying(), a parametric counterpart to the chained-equations sampler from #12. Each continuous time-varying covariate is modelled with a per-covariate linear mixed-effects transition model fitted to the observed first-order transitions: X_j(t) ~ X_1(t-1) + ... + X_p(t-1) + delta_time [+ time] + static + (1 | ID) Lagged values of all time-varying covariates capture both autocorrelation and between-covariate relationships; a subject-level random intercept captures within-subject correlation, with fresh random effects drawn per simulated subject and residual noise added at each timepoint. The `trend` argument switches between a pure Markov/AR transition ("previous") and adding an absolute-time fixed effect ("time"). Reuses the baseline sampling, observation-time profile, clone/propensity design-matching and update-matrix machinery introduced in #12, so only the transition engine differs. Robustness: - models fitted once and reused across replicates - failed lme fit falls back to OLS (warns); unfittable covariate is carried forward (warns) - random_intercept = FALSE uses OLS directly - na.exclude in prediction skips rows with missing predictors instead of erroring, leaving those covariates unchanged Adds the `lme_timevarying` dispatcher method, nlme to Imports, docs and a test file (mocked structural tests plus end-to-end tests through the real nlme path). Bumps version to 0.0.0.9013. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address two correctness bugs from review of #14: 1. A static categorical level that appears only in single-timepoint subjects is dropped from the transition data (those subjects contribute no transitions), so it is never a fitting-time factor level. The baseline sampler can still draw it, which made predict() abort the whole simulation with "factor has new levels". Now the fitting-time factor levels are stored on the models object and any unseen baseline level is mapped to NA at prediction time, so those rows carry forward instead of crashing. 2. predict.lme/predict.lm with na.action = na.exclude drop rows with missing predictors rather than padding them back to NA, so `mu + ranef + residual` silently recycled a short prediction vector against the full-length random-effect/residual vectors, writing values to the wrong subjects. Now we predict only on complete- predictor rows (via the stored predictor names) and leave the rest at their carried-forward value. Removed the incorrect na.exclude comment. Add regression tests: a categorical level seen only at baseline, and a direct simulate_lme_transition_step call with an NA lag predictor. Bump version to 0.0.0.9014. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add time-varying linear mixed-effects covariate sampler
…ng' into feature/anonymize-nonmem-dataset # Conflicts: # DESCRIPTION
ferx is a private repo; CI's repo-scoped GITHUB_TOKEN cannot install it, which hard-fails pak dependency resolution for pkgdown and test-coverage. ferx is only used via a requireNamespace-guarded ferx::ferx_simulate call and is mocked in tests, so it is not needed for CI or the test suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add NONMEM dataset anonymization function
roninsightrx
changed the title
Add copula-based covariate simulation (Zwep et al. 2024, CPT
doi:10.1002/cpt.3099) and obscuring/widening options for the existing
samplers.
New functions:
- sample_covariates_copulas(): cross-sectional vine-copula sampler
(rvinecopulib, KDE marginals) reproducing the observed covariate
dependence structure. Continuous covariates only.
- sample_covariates_copulas_timevarying(): longitudinal variant. Per
time-varying covariate, fits a random-effects polynomial trajectory,
fits a vine copula to the per-subject coefficients plus static
covariates, then reconstructs simulated trajectories. Supports degree,
family_set, selcrit, bw_mult, truncate, and output-level multiplicative
noise.
Existing samplers:
- sample_covariates_bootstrap() gains cat_covs (categorical-aware
conditional filtering) and noise (multiplicative log-normal jitter on
continuous covariates).
- sample_covariates_mice_timevarying() and
sample_covariates_lme_timevarying() gain baseline_method
("mice"/"bootstrap") and noise. Bootstrap baselines reproduce the
observed joint baseline distribution instead of shrinking it toward the
multivariate mean, addressing the too-narrow baseline distributions.
Shared apply_covariate_noise() helper; sample_covariates() dispatcher and
docs updated; rvinecopulib added to Suggests. Tests added for all new
behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Previously `noise` on sample_covariates_mice_timevarying() and sample_covariates_lme_timevarying() was applied only to the baseline draw (the first timepoint), so the bulk of the trajectory was unchanged and the argument appeared to do nothing. Move noise to the assembled long-format output via a shared apply_timevarying_noise() helper: time-varying covariates are jittered independently at every timepoint, static covariates once per subject (kept constant over time). sample_tv_baseline() no longer jitters; the standalone sample_covariates_bootstrap() keeps its own noise argument. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Round the sampled covariates and the simulated concentrations (DV) to `sigdig` significant figures via signif() to further obscure the exact values. Default 4; NULL disables. Applied before the lloq comparison so censoring uses the rounded concentrations. Categorical/non-numeric covariates are left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Optionally draw several covariate samples and keep the one whose distribution best matches the observed covariate data, before running the (single) PK simulation. - n_candidates (default 1 = unchanged): number of covariate samples drawn with derived reproducible seeds; the closest match is selected. - similarity (default "energy"): population-level energy distance between observed and simulated per-subject covariate-trajectory feature matrices (baseline, mean, slope and within-subject SD per continuous covariate; per-subject time fraction in each level per categorical covariate), standardized by the observed spread. Near-constant features are dropped with a relative threshold to avoid catastrophic cancellation. Scoring is distributional, not per-subject, so it does not pull individual simulated subjects toward specific real subjects. The selected candidate's score is returned as the "similarity_score" attribute. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add score_on to anonymize_dataset(), folding the PK simulation into the candidate loop. "concentration" (default) fully simulates every candidate and selects the one whose simulated (time, log-concentration) observation cloud is closest (energy distance) to the observed concentrations. "covariate" keeps the cheaper prior behaviour: score the covariate trajectories and simulate only the selected sample once. n_candidates = 1 is unchanged (single draw + simulate, no scoring). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace lloq/censoring with loq and blq_method = "remove"|"cens" (default "remove"). "remove" drops below-LOQ observation rows; "cens" keeps them, sets DV to 0, and flags them with CENS = 1 (0 otherwise) -- the prior "cens" path left the simulated DV in place. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ulas) Add a `method` argument selecting which time-varying covariate sampler resamples the covariates: "mice" (default), "lme", or "copulas". To make the LME and copula samplers usable in the anonymization pipeline, give both a `design_id_var` argument that records, per simulated subject, the observed subject whose observation-time design was assigned/cloned -- the `.design_id` bookkeeping that build_anonymized_simulation_input() needs to attach the dosing/observation events. Candidate sampling is routed through a shared draw_anonymize_candidate() dispatcher used by both the concentration- and covariate-scoring paths. "lme" and "copulas" support only continuous covariates and error early on categorical ones. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add copula covariate samplers; bootstrap baseline and noise options
1 participant
Summary
sample_covariates(method = "mice_timevarying")Tests
devtools::test()(396 passing, 0 failures)