Skip to content

feat: Support pre-computed attributions in run_modisco via method='completed'#187

Merged
avantikalal merged 3 commits intoGenentech:mainfrom
jsdearbo:pre-computed-attr-modisco
Mar 25, 2026
Merged

feat: Support pre-computed attributions in run_modisco via method='completed'#187
avantikalal merged 3 commits intoGenentech:mainfrom
jsdearbo:pre-computed-attr-modisco

Conversation

@jsdearbo
Copy link
Copy Markdown
Contributor

Overview

This PR adds support for passing pre-computed attributions to run_modisco by introducing method='completed'.

Currently, grelu.interpret.modisco.run_modisco requires computing attributions dynamically from the model during the function call. While convenient for smaller workflows, this tightly couples the heavy sequence-to-function model inference with the motif discovery step.

Motivation

For larger analysis pipelines, it is often necessary to decouple attribution generation from MoDISco execution. For example:

  • Compute constraints: Computing gradients for large models often requires heavy GPU allocation, while TF-MoDISco is typically run downstream on high-memory CPU nodes.
  • Iterative downstream analysis: Researchers often need to run MoDISco multiple times on different subsets of the data using different masking windows or flank sizes. Re-computing attributions for every subset via the existing run_modisco workflow is computationally redundant and slow.

By allowing method='completed', users can load an existing numpy array of attributions and pass them directly into the MoDISco pipeline via kwargs.

Changes Made

  • Updated src/grelu/interpret/modisco.py to bypass the internal attribution calculation when method='completed' is specified.
  • Added standard ValueError handling if the attributions kwarg is missing.
  • Added my name to AUTHORS.rst.
  • Ran pre-commit to ensure compliance with Black/Flake8 styling.

Impact

This is a non-breaking change. The default behavior for other methods (inputxgradient, saliency, etc.) remains exactly the same.

@avantikalal
Copy link
Copy Markdown
Collaborator

Hi @jsdearbo , this is a good idea but I would make a minor implementation change. Can we make "attributions" an optional parameter instead of expecting users to pass it in as **kwargs?

@jsdearbo
Copy link
Copy Markdown
Contributor Author

Hi @avantikalal,

Absolutely! I've updated run_modisco so attributions is now an explicit optional parameter, and I also synced my branch with the latest upstream/main.

Appreciate the suggestion, and I’m happy to make any other changes if needed.

@avantikalal avantikalal merged commit 7a523da into Genentech:main Mar 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants