Repository navigation
Fix comprehensiveness/sufficiency crash on code-sequence inputs - #1284
Merged
Merged
Conversation
RemovalBasedMetric._apply_ablation multiplied plain integer code-id tensors by a float mask, which turned them into floats, so nn.Embedding raised and comprehensiveness/sufficiency crashed for every model with sequence inputs (MLP, Transformer, RETAIN, ...). Only StageNet's tuple inputs took the integer-safe path. Integer inputs are now ablated by setting the removed codes to the padding id 0 with torch.where, keeping the dtype, for every ablation strategy. One helper serves both the tuple and plain-tensor paths, and keeps the existing guard that leaves one code per sample. Float inputs are unchanged. Revives the fix proposed in sunlabuiuc#1170 by @fbonc, which was closed as stale. Co-Authored-By: Claude Opus 5.5 <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.
Why
ComprehensivenessMetricandSufficiencyMetric(andevaluate_attribution/Evaluator) crash for any model whose inputs are plain code-sequence tensors: MLP, Transformer, RETAIN and so on.RemovalBasedMetric._apply_ablationhandles StageNet's(time, values)tuples with an integer-safe path, but plain tensors are ablated withx * (1 - mask). The float mask turns the code ids into floats, so the embedding lookup fails. The"mean"strategy has the same problem, and averaging code ids would be meaningless even if it ran.@fbonc proposed a fix for this in #1170, which was auto-closed as stale. This PR revives it. Credit to them for the diagnosis.
What
pyhealth/metrics/interpretability/base.py:_ablate_codessets the removed codes to the padding id 0 withtorch.where, so the dtype is kept. It also keeps the existing guard: a sample whose codes would all be removed keeps its first code.ablation_strategyis set to.docs/api/metrics/pyhealth.metrics.interpretability.rst: explains how integer inputs are ablated.examples/interpretability/comprehensiveness_code_sequences.py: trains an MLP on synthetic code sequences and scores Integrated Gradients. It runs in seconds on CPU with no downloads. It prints comprehensiveness 0.987 and sufficiency -0.006, because the label is driven by a single code.Tests
tests/core/test_interp_ablation_codes.py(5 tests):evaluate_attributionwith MLP and Transformer under all three strategies gives finite scores.masterand pass with this change.test_interp_metrics,test_interpret_ensemble,test_ig_gim,test_stagenet,test_stagenet_mha,test_shap,test_lime(178 tests, OK).tools/check_pr_rules.pypasses.🤖 Generated with Claude Code