Unify sparse and dense NCA parameters - #634
Open
billdenney wants to merge 14 commits into
Open
Conversation
Feasibility analysis for giving each registered parameter an optional sparse-specific estimator (FUN_sparse) with fallback to the dense FUN, collapsing the parallel sparse_* parameter vocabulary. Includes an appendix on why the sparse variance/df theory is tied to the linear trapezoidal rule and what a lin up/log down extension would require. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sparse PK reached the calculations through two passes over the split data: a dense pass over the arithmetic-mean profile and a second pass over the pooled individual samples, with each registered parameter computed only in the pass matching its `sparse` flag. Per-parameter dispatch needs both representations in scope at once, so the two passes become one. pk.nca() now runs a single purrr::pmap that carries `data_sparse_conc` alongside `data_conc`. pk.nca.intervals() filters and sorts both, and pk.nca.interval() gains `conc.sparse`/`time.sparse` (plus the group-level pair) in its source map beside the mean-profile `conc`/`time`. Routing still follows the registered flag: remap_sparse_sources() points a sparse-flagged parameter's resolved formals map at the pooled samples, and a sparse parameter is skipped when there are no pooled samples, as it was for dense PK before. The `sparse` argument is gone from both functions; any_sparse_dense_in_interval() stays as an internal helper and is now asked about both kinds. The output is unchanged, row order included. Results are marked with a "sparse" attribute as they are calculated so that pk.nca.intervals() can keep the two apart, and pk.nca() still reports every dense result before any sparse one. The imputation chain, factored out as impute_conc_time(), runs over the pooled samples too, so a sparse parameter sees the profile it saw when it had a pass of its own. The one visible change is a verbose message: the two "Starting dense/ sparse PK NCA calculations" messages become one "Starting PK NCA calculations", since there is now one pass. The per-interval "No dense/sparse calculations requested" messages are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pk.nca.interval() now holds the pooled individual samples for sparse PK alongside the arithmetic-mean profile, so "conc.sparse", "time.sparse", their group-level pair, and "subject" resolve like any other data source named in a formalsmap. Add them to the source list in add.interval.col(), saying that they are NULL with dense PK. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
For most NCA parameters the sparse method is the dense method applied to the arithmetic-mean profile, and that fallback has always been what sparse data got. A few parameters have a real sparse estimator instead, and until now those lived under a parallel vocabulary (`sparse_auclast` and the four parameters derived from it) that had to be maintained pairwise and that no other derived parameter could reach. add.interval.col() gains `FUN_sparse` and `formalsmap_sparse`: the calculation function and formals map to use when the data are sparse. pk.nca.interval() calls `FUN_sparse` when the pooled samples are in scope, resolving `formalsmap_sparse` and then pointing `conc`/`time` at the pooled samples the way a sparse-flagged parameter's are pointed, and reports the result among the sparse results. A parameter with no `FUN_sparse` still falls back to `FUN` on the mean profile. parameter_arg_spec() caches the two calling conventions separately. `auclast` and `aumclast` register the estimators, wrapping the existing sparse machinery and reporting under the unified names, with the new `auclast_se`/`auclast_df` and `aumclast_se`/`aumclast_df` companions (underscore suffixes, following `sparse_auc_se`). The values equal `sparse_auclast` and `sparse_aumclast`, which are unchanged, and every parameter derived from an AUC now picks the sparse estimate up by name: `cl.last` on sparse data equals `cl.sparse.last`, and the rest of the derived graph comes along without a hand-written sparse twin. Two guards go with it. A companion that only a sparse estimator produces would silently give nothing for dense data, so assert_intervals() refuses one instead; the older `sparse = TRUE` registrations keep being skipped rather than refused. And because the sparse variance theory needs the linear trapezoidal rule, pk.nca() says once per call that `auc.method` does not reach the estimators when the option asks for anything else. Dense results are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Record the behavior change (auclast and aumclast on sparse data now use the sparse estimators, with _se/_df companions, and no longer follow auc.method), the exported pk.nca.interval() signature change from the one-pass refactor, and the verbose start-message class collapse. The sparse vignette gains a short section showing the unified request path as the recommended one, noting that the derived-parameter graph now reaches sparse data and that the sparse_* names still work. The full rewrite waits for the deprecation. Note in the plan that the companion-name convention is settled on the underscore suffix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`auclast` and `aumclast` now carry the sparse estimators and every derived parameter reaches them by name, so the parallel vocabulary has nothing left to do. Requesting one of `sparse_auclast`, `sparse_auc_se`, `sparse_auc_df`, `sparse_aumclast`, `sparse_aumc_se`, `sparse_aumc_df`, `cl.sparse.last`, `mrt.sparse.last`, `kel.sparse.last`, `vss.sparse.last`, or `vz.sparse.last` in an interval specification warns once per session with the unified name to use, and will be an error in the next minor release. The values are unchanged this release. These are interval-specification columns rather than functions, so there is no call for lifecycle to attach itself to; the warning is a classed rlang condition raised where the intervals are validated. It names only the columns the specification itself wrote, not the deprecated dependencies they drag in, which is why interval_requested_params() gains an `expand` argument. Ten of the eleven replacements give the same number as the name they replace -- `kel.last`, like the rest of the `kel` family, is 1/MRT, so it matches `kel.sparse.last`. `vz.last` is the exception and the warning says so: it divides the clearance by the lambda.z fitted on the mean profile instead of by 1/MRT, which is what made `vz.sparse.last` numerically identical to `vss.sparse.last`. The functions behind the estimators (`pk.calc.sparse_auclast()`, `var_sparse_auc()`, `cov_holder()`, ...) are not deprecated: they are what the unified estimators call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… dense one A sparse context used to select only the parameters flagged sparse in the registry, which is now exactly the deprecated set, so the `sparse_single_dose` preset emitted names that warn. Sparseness is no longer one of the selection dimensions: a sparse design reports the same parameters as the dense design it corresponds to -- `auclast` and `aumclast` estimated with the sparse methods, the rest calculated from the arithmetic-mean profile -- and differs only in imputing nothing. What is never selected, in any context, is a parameter that *needs* sparse data. classify_sparse() now covers both kinds: the deprecated registrations it already found through the registry flag, and the standard errors and degrees of freedom that only a sparse estimator returns, which arrive as estimator output whether or not they were asked for. Those companions are added directly rather than through deps_union(), which reaches everything sharing a calculation function and would otherwise sweep in the parameter each one annotates along with its whole downstream family. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… sparse `auclast` calculated from the pooled samples and `auclast` integrated from one subject's profile are different estimators reported under one name, and CDISC distinguishes them (SPARSEAL against AUCLST). `pptestcd_cdisc` and `pptest_cdisc` gain a sparse-keyed form, `list(sparse = list(dense = ..., sparse = ...))`, alongside the existing route-keyed one, and resolve_cdisc_value() picks between them. The consumer has the context it needs: pknca_cdisc_translate() holds the PKNCAresults, a PKNCAconc is entirely sparse or entirely dense, and a parameter with a FUN_sparse used it for every row of a sparse analysis, so sparseness is settled once for the whole result rather than per row. Unlike routes, the keys are a closed set, so both must be given. `auclast` gets AUCLST/SPARSEAL and the matching test names; `aumclast` gets only the test name, because CDISC has no separate code for a sparsely estimated AUMClast. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite the sparse vignette around the standard parameter names: the only difference from a dense analysis is `sparse = TRUE` on `PKNCAconc()`, the derived-parameter section is the ordinary derived graph rather than five hand-maintained twins, and the deprecated names move to a table saying what to use instead and that they become errors in the next minor release. The Vz change gets its own explanation. `vz.last` divides the clearance by the lambda.z fitted on the mean profile, which the example data cannot fit -- so it is NA there, where the retired `vz.sparse.last` always produced a number by dividing by 1/MRT and thereby made Vz identical to Vss. `kel.last` is 1/MRT like the rest of the family, so it is unchanged; `lambda.z` is what to ask for instead. NEWS records the deprecation with its removal release, the Vz semantics, the preset change, and the sparse-keyed CDISC mapping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ca-feasibility-68242b # Conflicts: # NEWS.md
An imputed measurement belongs to no animal, but pk.nca.interval() ran the imputation chain over the pooled samples as well as over the mean profile, leaving `subject` one element short of `conc`. Before the as_sparse_pk() subject check became a real assertion this quietly recycled into the covariance bookkeeping; since then it dies with an internal assertion failure asking for a bug report. The combination is now refused where it is diagnosed, with a pknca_error_sparse_impute naming the interval, the imputation method, and the parameters that would have been calculated from the pooled samples. It is refused only when the imputation actually changed those samples: one that finds a concentration already measured at the interval start changes nothing and still calculates, and one requested alongside only mean-profile parameters was never a problem. The mean-profile imputation path is untouched, and dense analyses are unaffected. Which function calculates a parameter, whether it reads the pooled samples, and whether it can run at all move into parameter_dispatch(), so that the new check and the calculation loop cannot disagree about what counts as a sparse calculation. Imputation that a sparse estimator could use -- a known point with zero variance, say -- is a statistical question, not a plumbing one, and is left for later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`FUN_sparse` made the `sparse` flag redundant: a parameter that only sparse data can produce is exactly one with a sparse estimator and no dense function. The flag is retired. `add.interval.col(sparse = TRUE)` is an error pointing at the replacement pattern, an explicit FALSE is still accepted, and registry entries no longer carry the field. The seven flagged parameters are re-registered that way -- `FUN = NA` with their old function as `FUN_sparse` and their old formals map as `formalsmap_sparse` -- and everything that read the flag now derives it: the calculation loop (where the skip condition collapses to "has a function to call" and a sparse result is exactly one the estimator produced), any_sparse_dense_in_interval(), classify_sparse(), sparse_only_params(), and fun_sparse_params(). Two readers had to learn that a sparse-only parameter's calculation function is its estimator, because both used to read `FUN` directly and would otherwise have seen `NA`: get.parameter.deps() would have hit its "no function and multiple dependencies" abort for mrt/vss/vz.sparse.last, and parameter_direct_refs() would have lost the dose that makes cl.sparse.last uncalculable without dosing. interval_col_fun() and interval_col_formalsmap() answer that once for everyone. Two behaviors are held fixed. Requesting a deprecated name for dense data is still a silent skip, so the dense-data guard refuses the sparse-only parameters that are not deprecated -- today the four `_se`/`_df` companions, and any newly registered sparse-only parameter, which is refused rather than skipped now that the two are the same kind of thing. And the values, row order, PPANMETH, classification table, CDISC codes, and interval-table presets are unchanged. The note that `auc.method` does not reach the sparse estimators now names only the areas, since the seven newly-estimator-bearing parameters include clearances and residence times that the option never described. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The plan sketched `lifecycle` deprecation with interval-spec translation and said Vz and Kel both change. As built, the deprecation is a classed warning raised where interval requests are validated -- these are interval-specification columns, not functions -- nothing is translated, and only Vz changes value. Retiring the `sparse` argument followed from `FUN_sparse` making the flag derivable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The parameters-available chunk in v03 assumed every parameter with FUN = NA names at most one dependency; the sparse-only re-registrations made mrt/vss/vz.sparse.last FUN = NA with two dependencies each, so the vapply building the table failed with a length-2 result and broke the vignette build on every R CMD check platform. The helper now reports the sparse estimator as the calculation function for a sparse-only parameter and collapses multiple dependency names into one string. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the sparse/dense parameter unification designed in
design/sparse-dense-unification-plan.md(included in this PR), in threephases:
arithmetic-mean profile and the pooled individual samples) in scope per
interval; output is byte-identical to the two-pass engine.
FUN_sparsedispatch: on sparse data,auclastandaumclastareestimated with the Bailer point estimate and Nedelman-Jia/Holder standard
error under their standard names, returning new
auclast_se/auclast_df(and
aumclastequivalents). Every AUC-derived parameter (cl.last,mrt.last, dose-normalized variants, ...) picks the sparse estimate up byname. Requesting an
_se/_dfcompanion on dense data is an error;pk.nca()notes once per call that the sparse estimators arelinear-trapezoidal only when
auc.methodasks for anything else.sparse_auclastfamily and*.sparse.last) still calculate with identical values but warn once persession, and will be an error in the next minor release. Presets and
pknca_interval_table(sparse = TRUE)now select the same parameters as thematching dense design. CDISC codes gain a sparse-keyed mapping
(
AUCLST/SPARSEAL). The sparse vignette is rewritten around the unifiednames.
Behavior changes
auclaston sparse data is now the sparse estimate (linear trapezoidal onthe mean concentrations) rather than
auc.methodapplied to the meanprofile, and brings
auclast_se/auclast_dfwith it.vz.laston sparse data uses the mean-profilelambda.z, not the 1/MRTchain that made the deprecated
vz.sparse.lastequalvss.sparse.last.pk.nca.interval()lost itssparseargument and gainedpooled-sample arguments.
Verification
pk.nca()output across 9 scenarios.sparse_*values.Notes for review
aumclastkeepsAUMCLSTandis distinguished by PPTEST only; the mapping supports a code when one exists.
R/sparse.R/test-sparse.R, as does the in-flightas_sparse_pk()subject-validation fix; merge one, then rebase the other.🤖 Generated with Claude Code