add Gram matrix fast path for ZnS computation - #25
Conversation
There was a problem hiding this comment.
I tested this on full Ag1000G chromosome 3L data (2,940 haplotypes x 8,248,442 variants) and it OOMs:
cupy.cuda.memory.OutOfMemoryError: Out of memory allocating 97,001,678,336 bytes
(allocated so far: 48,533,833,216 bytes)
The issue is that _prepare_segregating creates the full haplotype matrix as float64, and then the standardization step S = (hap_clean - p) * inv_sqrt_pq creates another full-size (n_hap x m) float64 array. At 2940 x ~8M segregating sites, that's ~185GB for S alone.
The Gram trick itself seems cool -- K = S @ S.T is only 2940 x 2940 -- but you need to accumulate K in chunks over columns (sites) so that only a (n_hap x chunk_size) slice of S is ever in memory at once. Something like:
K = cp.zeros((n_hap, n_hap), dtype=cp.float64)
for col_start in range(0, m, chunk_size):
S_chunk = (hap_clean[:, col_start:col_end] - p[col_start:col_end]) * inv_sqrt_pq[col_start:col_end]
K += S_chunk @ S_chunk.TThe existing _memutil.py chunking infrastructure may be useful here.
Also: the missing data fallback to the tiled path is correct, but means the fast path won't activate on datasets with any missingness. This isn't ideal -- users with real-world data (which commonly has missing genotypes) won't see the speedup unless they exclude sites and it means we have to keep a fallback code path in the project-- more code to maintain.
Please test on full-scale data (e.g., 1000 Genomes or Ag1000G full chromosome arms) before resubmitting. The unit tests all pass at small scale but the memory issue only shows up with real genomic data. Also please deal with the missing data issue so that this could replace the existing code fully.
|
Hi Andy, For missing data: the Gram path now handles it via mean imputation (S=0 for missing entries) with a MCAR correction. For small m where tiled is fast anyway it auto-selects the exact path, and a Tested on full Ag1000G 3L: 2940 x 8.2M runs in 14.5s (vs ~5 hours for the tiled path at that scale), ~3GB GPU overhead, matches tiled at machine precision on a 50K subset. 416/416 tests pass. Full details in the commit message. |
|
Thanks for the revision Kevin -- the chunked Gram accumulation is a great improvement and the OOM issue is resolved. I ran some validation on real unphased Ag1000G data (3L, ~2940 haplotypes, 74% missingness) comparing the PR branch against main:
There are two problems:
So the missing data path needs to be tightened up. Rather than reimplementing the missing data filtering, use the existing |
cabfc8d to
9a12a28
Compare
|
Thanks, Andy. I reproduced this on the unphased gamb 3L Ag1000G data and fixed both failure modes. The validation table below is from this exact input:
Current behavior:
What was wrong:
What I changed:
Here is the comparison on that exact dataset and region:
One additional note on performance: I also compared the fixed exclude Gram path to the exact tiled reference on the same retained complete sites. On this dataset, exclude leaves relatively few complete I also added targeted ZnS regressions for:
I reran the targeted ZnS tests on poppy after the fix and they pass. |
|
I think your proposal from slack is the right approach. Thinking it through: The mixed-n problem in Proposed design (2 paths, no
c1, c2, c3, c4, n = _tile_counts(hi, vi, hj, vj)
p_i = (c1 + c2) / n
p_j = (c1 + c3) / n
D = c1 / n - p_i * p_j
denom = p_i * (1 - p_i) * p_j * (1 - p_j)
r2_tile = cp.where((denom > 0) & (n >= 2), D**2 / denom, 0.0)
This eliminates the |
|
also note-- you should rebase before you start editing |
fd32b5f to
50ff091
Compare
|
Sorry, about the delay. Rebased onto the latest main and simplified the implementation as discussed. For estimator='r2', missing-data include now uses exact pairwise-complete counts, while complete/exclude data uses the chunked Gram fast path. sigma_d2 remains unchanged. |
|
This is also getting rewritten as part of #180 (that's in progress). Can you wait on these two PRs until the multiallelic rewrite is done and in main? They're almost certain to conflict. I can ping you then. |
|
Sure, no problem -- they can wait of course |
Summary
||K||_F^2
Benchmarks (NVIDIA A100, 200 haplotypes x 50k variants)
Also speeds up
zx(callszns3x internally).Test plan
pixi run pytest tests/ -k 'zns or ld or omega or windowed'(68 passed)pixi run python debug/bench_zns.pyfor speed + correctness comparison