From 0c3a5ad6f29ac3f0fc9fcf8987d61047da7f232a Mon Sep 17 00:00:00 2001 From: Robrecht Cannoodt Date: Thu, 30 Jul 2026 09:19:04 +0200 Subject: [PATCH] trim the comments that were review notes The recent bug fixes left comments explaining why the change was correct, which is PR material rather than something the next reader of the code needs. Keep the one-line narrating comments, drop the justifications. --- src/data_processors/process_dataset/script.R | 8 +++----- src/methods/lm/script.R | 2 +- src/methods/novel/novel_predict/script.py | 2 -- src/metrics/correlation/script.R | 7 ++----- src/metrics/mse/script.py | 3 +-- 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/data_processors/process_dataset/script.R b/src/data_processors/process_dataset/script.R index e0a8e2a9..1ab7a2c1 100644 --- a/src/data_processors/process_dataset/script.R +++ b/src/data_processors/process_dataset/script.R @@ -90,8 +90,7 @@ if (ad2_mod == "ATAC") { # subset to make the task computationally feasible if (ncol(ad2) > 10000) { poss_ix <- which(Matrix::colSums(ad2$layers[["normalized"]]) > 0) - # sample by position -- sample(x, n) errors when n > length(x), and treats a - # length-1 x as seq_len(x) + # sample by position -- sample(x, n) treats a length-1 x as seq_len(x) sel_ix <- sort(poss_ix[sample.int(length(poss_ix), min(10000, length(poss_ix)))]) ad2 <- ad2[, sel_ix]$copy() ad2_var <- ad2_var[sel_ix, , drop = FALSE] @@ -109,9 +108,8 @@ if ("is_train" %in% colnames(ad1$obs)) { is_train <- which(ad1$obs[["is_train"]] == "train") is_test <- which(ad1$obs[["is_train"]] != "train") } else { - # No predefined split -- obs['is_train'] carries the NeurIPS 2021 competition - # split and other datasets have no reason to have it. Hold out a quarter of the - # batches instead, so the test cells come from donors the method has not seen. + # no predefined split -- hold out a quarter of the batches, so the test cells + # come from donors the method has not seen batches <- unique(as.character(ad1$obs[["batch"]])) if (length(batches) > 1) { test_batches <- sample(batches, max(1, floor(length(batches) / 4))) diff --git a/src/methods/lm/script.R b/src/methods/lm/script.R index 8ffef929..9ced5385 100644 --- a/src/methods/lm/script.R +++ b/src/methods/lm/script.R @@ -31,7 +31,7 @@ ix <- seq_len(nrow(input_train_mod1)) dr_train <- dr[ix, , drop = FALSE] dr_test <- dr[-ix, , drop = FALSE] -# add an intercept column -- fastLm() uses the design matrix as is +# add an intercept column dr_train <- cbind(intercept = 1, dr_train) dr_test <- cbind(intercept = 1, dr_test) diff --git a/src/methods/novel/novel_predict/script.py b/src/methods/novel/novel_predict/script.py index cbc47b77..ddc0233c 100644 --- a/src/methods/novel/novel_predict/script.py +++ b/src/methods/novel/novel_predict/script.py @@ -95,8 +95,6 @@ dataloader_test = DataLoader(dataset_test, 32, shuffle = False, num_workers = 4) outputs = [] -# the weights are loaded with map_location='cpu', so move the model onto the device -# we selected above -- otherwise this component requests a GPU and never uses it model = model.to(device) model.eval() with torch.no_grad(): diff --git a/src/metrics/correlation/script.R b/src/metrics/correlation/script.R index 1050f1ff..79cbbd3d 100644 --- a/src/metrics/correlation/script.R +++ b/src/metrics/correlation/script.R @@ -39,9 +39,7 @@ pv_sd2 <- proxyC::colSds(pv) tv_sd1 <- proxyC::rowSds(tv) pv_sd1 <- proxyC::rowSds(pv) -# correlate matching rows (margin 1) or columns (margin 2) of two matrices. -# dynutils::calculate_similarity() would do this too, but it ends in as.matrix(), -# densifying an n x n / p x p matrix just so we can read its diagonal. +# correlate matching rows (margin 1) or columns (margin 2) of two matrices paired_similarity <- function(x, y, margin, method) { if (margin == 1) { x <- Matrix::t(x) @@ -87,8 +85,7 @@ mean_spearman_per_gene <- mean(spearman_vec_2) tv_vec <- as.vector(tv) pv_vec <- as.vector(pv) -# a constant prediction has no correlation to speak of -- score it as 0, the same -# substitution the per-cell and per-gene metrics make above. cor() would return NA. +# zero variance -- score as 0, like the per-cell and per-gene metrics above if (sd(tv_vec) == 0 || sd(pv_vec) == 0) { overall_pearson <- 0 overall_spearman <- 0 diff --git a/src/metrics/mse/script.py b/src/metrics/mse/script.py index 2ca9fc68..703f6226 100644 --- a/src/metrics/mse/script.py +++ b/src/metrics/mse/script.py @@ -26,8 +26,7 @@ logging.info("Computing MSE metrics") -# coerce to sparse -- a method is free to return a dense layer, and -# sparse - dense yields a np.matrix, which has no .power() +# coerce to sparse -- sparse minus dense yields a np.matrix, which has no .power() tmp = csr_matrix(ad_sol.layers["normalized"]) - csr_matrix(ad_pred.layers["normalized"]) rmse = np.sqrt(tmp.power(2).mean()) mae = np.abs(tmp).mean()