Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions R/run_ML.R
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,17 @@ createMLinputList <- function(path,

path <- normalizePath(path)

# if (isTRUE(LOO) && (is.null(stratify_by) || !(stratify_by %in% c("year", "country")))) {
# stop("For Leave-One-Out (LOO) models, stratify_by must be 'year' or 'country'.")
# }
# Leave-one-drug-out cross-testing (LOO + cross_test) is a separate mode
# that doesn't stratify by year/country - see the `cross_test && LOO`
# branch below. Stratified LOO (leave-one-year/country-out) always needs
# stratify_by, whether or not it's also cross-tested.
if (isTRUE(LOO) && !isTRUE(cross_test) &&
(is.null(stratify_by) || !(stratify_by %in% c("year", "country")))) {
stop(
"For Leave-One-Out (LOO) models without cross-testing, ",
"stratify_by must be 'year' or 'country'."
)
}

if (isTRUE(MDR) && (!is.null(stratify_by) || LOO || cross_test)) {
stop("MDR can only run when stratify_by = NULL, LOO = FALSE, cross_test = FALSE.")
Expand Down Expand Up @@ -579,7 +587,9 @@ parsed_drugs <- parsed |>
out_top = paths$ML_top_features,
out_models = paths$ML_models,
out_pred = paths$ML_prediction
)
)

return(out)
}
# LOO requires special directory structure resolution
test_path <- file.path(path, stringr::str_remove(basename(paths$matrix_path), "^LOO_"))
Expand Down Expand Up @@ -1002,7 +1012,12 @@ runMLmodels <- function(path,
MDR = FALSE,
cross_test = cross_test
)


if (nrow(files) == 0) {
message("No files found to process. Exiting.")
return(invisible(NULL))
}

.findNonRanPrefixes <- function(files,
seed,
shuffle_labels = FALSE) {
Expand Down
58 changes: 58 additions & 0 deletions tests/testthat/test-run-ml-models.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Regression tests for control-flow bugs in
# createMLinputList()/runMLmodels().

test_that("runMLmodels() exits cleanly instead of crashing on no files", {
tmp <- withr::local_tempdir()
# No matrix parquet files are created, so createMLinputList() returns an
# empty tibble. runMLmodels() must message and return early instead of
# trying to use the empty result (which previously crashed with
# "a character vector argument expected").
result <- NULL
expect_message(
result <- runMLmodels(
path = tmp, stratify_by = NULL, LOO = FALSE, cross_test = FALSE
),
"No files found"
)
expect_null(result)
})

test_that("createMLinputList() rejects LOO w/o year/country (no cross-test)", {
tmp <- withr::local_tempdir()
expect_error(
createMLinputList(tmp, LOO = TRUE, cross_test = FALSE, stratify_by = NULL),
"stratify_by must be"
)
})

test_that("createMLinputList() allows LOO+cross-test with stratify_by = NULL", {
# LOO + cross_test with stratify_by = NULL is a distinct mode
# (leave-one-drug-out cross-testing, "Case A" in the cross_test && LOO
# branch) and is intentionally exempt from the year/country requirement
# above.
tmp <- withr::local_tempdir()
paths <- createMLResultDir(
tmp,
stratify_by = NULL, LOO = TRUE, cross_test = TRUE, MDR = FALSE
)

# A single matrix filename that satisfies both the general filename parser
# and the LOO-specific parser used inside Case A. File content is
# irrelevant - createMLinputList() only inspects filenames.
file.create(file.path(
paths$matrix_path,
"Csp_drug_leaveout_leaveout_genes_binary_sparse.parquet"
))

out <- createMLinputList(
tmp,
stratify_by = NULL, LOO = TRUE, cross_test = TRUE, MDR = FALSE
)

# Before the return() fix, Case A built the right result but never
# returned it, so execution fell through into the stratify_by != NULL
# branch, which self-joins the single file against itself and always
# filters it out (ref_file != test_file), silently returning 0 rows.
expect_equal(nrow(out), 1)
expect_true(grepl("_drug_leaveout_", out$output_prefix))
})