Fix EnsembleLightningModel VEP: run models independently to avoid allele/variant dimension scrambling - #57
Merged
Conversation
Two bugs caused wrong variant effect predictions when using the ensemble model: Bug 1 — batch_size=1 allele scrambling: EnsembleLightningModel.predict_step concatenates 4 model outputs along dim=0 per batch. With batch_size=1, ref and alt alleles land in separate batches, so after concatenation the tensor becomes [m0_ref, m1_ref, m2_ref, m3_ref, m0_alt, ...] instead of the expected [ref, alt] pairing. The subsequent rearrange in predict_on_dataset then computes cross-model differences instead of alt−ref LFC. Bug 2 — n_seqs > 1 model/variant dimension swap: With batch_size >= n_alleles (correct allele pairing), when a variant overlaps multiple genes the LFC tensor is ordered variant-outer: [g0_m0, g0_m1, g0_m2, g0_m3, g1_m0, ...]. The rearrange '(e b) t -> e b t' with e=n_models treated model as the outer dimension, so it sliced off the wrong blocks and mixed LFCs from different genes and models when computing the ensemble mean and when populating the save_replicates per-replicate columns. Both bugs are fixed by overriding predict_on_dataset in EnsembleLightningModel to run each constituent model's predict_on_dataset independently, then stack and average. This matches the documented semantics of the ensemble and produces values identical to running each replicate individually and averaging manually. The fix also simplifies the code: the broken (e b) rearrange is replaced by an explicit np.stack. Gene expression prediction (n_alleles=1) was unaffected by these bugs. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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.
Problem
predict_variant_effectwithmodel='ensemble'produced wrong log-fold-change predictions. The ensemble mean did not match the arithmetic mean of the four constituent replicate models run individually.Confirmed by running rs2158799 (chr7:28237488 C>G) against JAZF1 in blood cell types:
model='ensemble'(broken)Root cause: two bugs in
EnsembleLightningModel.predict_on_datasetBug 1 — batch_size=1 allele scrambling
EnsembleLightningModel.predict_stepconcatenates the outputs of all 4 models alongdim=0within each batch. With the defaultbatch_size=1, ref and alt alleles land in separate batches. After model-concatenation the flattened tensor becomes:The subsequent
rearrange("(b n a) t -> b n a t", a=2)inLightningModel.predict_on_datasetthen groupsm0_refwithm1_refas the two "alleles" for a single variant, and the LFCexpression[:,:,1,:] - expression[:,:,0,:]computes cross-model differences instead of alt−ref.Bug 2 — n_seqs > 1 model/variant dimension swap
With
batch_size=n_alleles=2(which does pair alleles correctly), the LFC tensor after all batches are collected is ordered variant-outer:EnsembleLightningModel.predict_on_datasetthen appliedrearrange("(e b) t -> e b t", e=4), which assumed model-outer ordering. Withn_seqs=3genes this sliced off the wrong blocks, mixing LFCs across genes and models. The ensemble mean and thesave_replicatesper-replicate columns were both wrong.Bug 2 only manifests when a variant overlaps more than one gene (
n_seqs > 1). Gene expression prediction (n_alleles=1) was not affected by either bug.Fix
Replace the
super().predict_on_dataset()+ broken rearrange with a loop that runs each constituent model'spredict_on_datasetindependently, then stacks and averages:Each
LightningModel.predict_on_datasetcorrectly handles allele ordering for anybatch_sizeand anyn_seqs. The explicitnp.stackreplaces the incorrect(e b) t -> e b trearrange.Test
Added
test_EnsembleLightningModel_predict_on_dataset_matches_individual_replicatestotests/test_lightning.py. The test mockspredict_on_dataseton each constituent model (so no GPU forward passes needed) and asserts:ensemble_preds[i]matches modeli's predictions exactly.The test runs in ~5 seconds and does not require a long-running mark.
Verification
🤖 Generated with Claude Code