Skip to content

Sparse AUMC#518

Open
PavanLomati wants to merge 3 commits intohumanpred:mainfrom
PavanLomati:Sparse-AUMC
Open

Sparse AUMC#518
PavanLomati wants to merge 3 commits intohumanpred:mainfrom
PavanLomati:Sparse-AUMC

Conversation

@PavanLomati
Copy link
Copy Markdown
Contributor

I added the sparse AUMC function along with five AUC-variant PK parameters. When running the tests, I encountered the following error: Failure (test-pk.calc.all.R:789:5): pk.nca can be run for each parameter independently (#473) all(is.na(param_res$result$PPORRES)) is not FALSE
actual: TRUE
expected: FALSE
The parameter mrt.sparse.last was not being calculated independently. To address this, I added the sparse-related parameters to test-pk.calc.all.R under non_pknca_covered_params: f, time_above, mrt.md.obs, mrt.md.pred,
sparse_auclast, sparse_auc_se, sparse_auc_df,
sparse_aumclast, sparse_aumc_se, sparse_aumc_df,
vss.md.obs, vss.md.pred, ceoi,
cl.sparse.last, mrt.sparse.last, vz.sparse.last,
vss.sparse.last, kel.sparse.last
Please let me know if this approach looks correct. After this update, the test results are:
[ FAIL 0 | WARN 4 | SKIP 0 | PASS 2264 ]
Remaining warning:
Warning (test-PKNCA.options.R:440:3): PKNCA.set.summary input checking reset = TRUE is not intended for general use; summary() may not work after resetting summary instructions.

I added the sparse AUMC function along with five AUC-variant PK parameters. When running the tests, I encountered the following error:
Failure (test-pk.calc.all.R:789:5): pk.nca can be run for each parameter independently (humanpred#473)
all(is.na(param_res$result$PPORRES)) is not FALSE
actual: TRUE
expected: FALSE
The parameter mrt.sparse.last was not being calculated independently.
To address this, I added the sparse-related parameters to test-pk.calc.all.R under non_pknca_covered_params:
f, time_above, mrt.md.obs, mrt.md.pred,
sparse_auclast, sparse_auc_se, sparse_auc_df,
sparse_aumclast, sparse_aumc_se, sparse_aumc_df,
vss.md.obs, vss.md.pred, ceoi,
cl.sparse.last, mrt.sparse.last, vz.sparse.last,
vss.sparse.last, kel.sparse.last
Please let me know if this approach looks correct.
After this update, the test results are:
[ FAIL 0 | WARN 4 | SKIP 0 | PASS 2264 ]
Remaining warning:
Warning (test-PKNCA.options.R:440:3): PKNCA.set.summary input checking
reset = TRUE is not intended for general use; summary() may not work after resetting summary instructions.
@PavanLomati PavanLomati requested a review from billdenney March 17, 2026 03:08
non_pknca_covered_params <- c(
"f", "time_above", "mrt.md.obs", "mrt.md.pred", "sparse_auclast", "sparse_auc_se", "sparse_auc_df",
"vss.md.obs", "vss.md.pred", "ceoi"
"f", "time_above", "mrt.md.obs", "mrt.md.pred",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ensure that all of these parameters are covered and not add to this list.

#' pharmacokinetics. Journal of Biopharmaceutical Statistics. 1998;8(2):317-328.
#' doi:10.1080/10543409808835241
#' @export
var_sparse_auc <- function(sparse_pk) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove this function?

R/sparse.R Outdated
options=options,
auc.type="AUClast",
lambda.z=NA
conc = conc, time = time, subject = subject, ...,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many whitespace-only changes that make the code harder to review. Please remove the whitespace changes.

#' doi:10.1080/10543409808835241
#' @keywords internal
#' @export
var_sparse_aumc <- function(sparse_pk) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you actually just moved the function down here. Please move it back so that the diff is easier to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants