Fix OOM in scm_gene_stat on large datasets (#98) - #99
Open
aviezerl wants to merge 1 commit into
Open
Conversation
quant was ceiling(nrow(mat_fg)/mc_cores), so the gene chunk size grew as mc_cores shrank. Each chunk densified three matrices (mat_fg, mat_ds, mat_n), giving quant x n_cells x 8B each; on 80k cells that is multi-GiB per chunk, multiplied by every parallel fork. Lowering mc_cores made it worse rather than better. Chunk on a fixed ~50MB dense block instead, so peak memory per fork no longer depends on mc_cores. Fixed-size chunks can leave a 1-gene tail, so the three row subsets now use drop=FALSE. sz_cor had a second OOM: apply(mat_fg[,subs], 1, cor) coerces the whole sparse submatrix to dense (genes x 50k cells x 8B). Replaced with .sparse_row_cor, a sums-of-products Pearson that needs one sparse matvec and never densifies. Also hard-stop if the sequential retry is still short instead of returning a truncated gene_stat, matching scm_downsamp. Verified by golden-master: gene_stat is identical() to the previous implementation on 800x20000 (chunk count changes 4 -> 3), 300x50001 (the >50k sz_cor branch) and 313x20000 (1-gene tail chunk). Claude-Session: https://claude.ai/code/session_01MFLoUB1VZFB5qRNYR6BxVL
aviezerl
force-pushed
the
fix-gstat-oom-98
branch
from
August 25, 2026 09:43
0bf09e5 to
e44f507
Compare
aviezerl
force-pushed
the
fix-gstat-oom-98
branch
from
August 25, 2026 12:14
e44f507 to
0bf09e5
Compare
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
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.
Fixes #98.
Chunking
quantwasceiling(nrow(mat_fg)/mc_cores), so the gene chunk size grew asmc_coresshrank. Each chunk densified three matrices (mat_fg,mat_ds,mat_n) atquant x n_cells x 8Beach, multiplied by every fork.Now chunks on a fixed ~50MB dense block, so peak memory per fork is independent of
mc_cores. Fixed-size chunks can leave a 1-gene tail, so the three row subsets usedrop=FALSE.I did not cap the workers at 3 as the issue suggests -
mc_coresis the user's setting, and with 50MB blocks the cap isn't needed.sz_cor
apply(mat_fg[,subs], 1, cor)coerces the whole sparse submatrix to dense. Replaced with.sparse_row_cor, a sums-of-products Pearson using one sparse matvec, no densification. Chose this over chunking theapplybecause it also removes a per-gene sort over all cells.Verification
Golden-master:
gene_statisidentical()to the previous implementation on>50000sz_cor branch.sparse_row_coralso matchesapply(mat, 1, cor)to 3 decimals including the zero-variance ->NAcase. Not covered: the OOM itself at 80k cells, which needs the real dataset.Not done
A fully sparse
stat_on_genes(viasparseMatrixStats) would remove the densification and the parallelism entirely, and retire the recurring parallel-chunking bug family (#71, #37, #96). That's a refactor with a new Bioconductor dependency, out of scope for a bug fix here.https://claude.ai/code/session_01MFLoUB1VZFB5qRNYR6BxVL