Repository navigation
Fix output shape mismatch in deep_lift_shap convergence check - #203
Open
avantikalal wants to merge 1 commit into
Open
avantikalal wants to merge 1 commit into
avantikalal wants to merge 1 commit into
Conversation
gReLU models pooled to a single value per task return shape (N, tasks, 1), not (N, tasks). tangermeme.deep_lift_shap indexes a single task with `y[:, target]`, which only consumes the tasks axis and leaves the trailing 1 in place. The resulting shape mismatch between its output-difference (N/2, 1) and attribution-sum (N/2,) tensors is then silently broadcast into a meaningless (N/2, N/2) matrix instead of raising a shape error, corrupting its internal convergence-delta check and producing spurious/inflated "Convergence deltas too high" warnings. Wrap the model passed to deep_lift_shap in a small adapter (_SqueezeOutput) that drops a trailing length-1 axis, restoring the flat (N, tasks) shape deep_lift_shap's convergence check expects. Models with genuine multi-position outputs (shape (N, tasks, L>1)) are left untouched. Verified against a real trained Enformer-based checkpoint (Genentech/human-atac-catlas-model): the same convergence deltas that previously appeared as a bogus (N, N) matrix with values up to ~10 now come out correctly shaped and an order of magnitude smaller. Fixes #202 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
avantikalal
added a commit
that referenced
this pull request
Sep 30, 2026
Wrap the model in _SqueezeOutput for the Enformer deepshap path too, so its convergence-delta check isn't corrupted by the same shape mismatch fixed for the general case in #202/#203. Without this, the Attention/AttentionPool/GELU hooks added here couldn't be evaluated against a trustworthy convergence signal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
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.
Summary
(N, tasks, 1), not(N, tasks).tangermeme.deep_lift_shapselects a single task viay[:, target], which only consumes thetasksaxis and leaves the trailing1in place.(N/2, 1)and its attribution-sum tensor(N/2,)is then silently broadcast by PyTorch into a meaningless(N/2, N/2)matrix instead of raising a shape error — corruptingdeep_lift_shap's own internal convergence-delta sanity check and producing spurious/inflated"Convergence deltas too high"warnings._SqueezeOutput, a smallnn.Moduleadapter that drops a trailing length-1 axis before the model output reachesdeep_lift_shap. Models with genuine multi-position outputs ((N, tasks, L>1)) are left untouched.ConvHead-based model using the default pooling, so it's independent of Enable deepshap for enformer models #56/Enable DeepSHAP for Enformer models #201.Fixes #202
Verification
Genentech/human-atac-catlas-model): the same convergence deltas that previously appeared as a bogus(N, N)-shaped matrix with values up to ~10 now come out correctly shaped (matching the expected per-reference batch sizes) and roughly an order of magnitude smaller.test_squeeze_output, a deterministic unit test of the wrapper (with and without a trailing singleton dim), since the existingtest_get_attributionsmodel's warning behavior turned out to be data-dependent and didn't reliably discriminate the bug on its own.test_get_attributionsto assert no"Convergence deltas too high"warning fires.tests/test_interpret.pysuite passes (12 passed, no regressions).Test plan
pytest tests/test_interpret.pypassestest_squeeze_outputpasses and fails if the wrapper is removed (verified by temporarily reverting thescore.pychange)🤖 Generated with Claude Code