model: SENAVAE - #3571
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (62.88%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3571 +/- ##
==========================================
- Coverage 88.58% 87.55% -1.03%
==========================================
Files 241 246 +5
Lines 23744 24749 +1005
==========================================
+ Hits 21033 21670 +637
- Misses 2711 3079 +368
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ori-kron-wis
left a comment
ori-kron-wis
left a comment
There was a problem hiding this comment.
Let's have the external folder name in lower case to match other external models.
Add the class model name to the index init file of the external folder.
Add Tutorial(s) to scvi-tutorials
Add model description md file
Add Unit tests (most important)
Add changlog
| adata : AnnData | ||
| Annotated data object containing single-cell expression data with perturbation annotations. | ||
| Must be pre-registered via setup_anndata with control/perturbation labels. | ||
| go_file_path : str |
There was a problem hiding this comment.
Those 2 go-file links and gene_symb file as mandatory inputs to the model will be untrivial to manage, as it doesn't exist for any of the other models. You will have to provide a function that checks for the correctness of those files.
You will have to provide a full description of what those files are and their structure.
You will have to upload such data for the unit-test example.
| _data_splitter_cls = SENADataSplitter # Control-perturbation aware data splitting | ||
|
|
||
| @staticmethod | ||
| def set_seeds(seed: int) -> None: |
There was a problem hiding this comment.
I think we can use the generic src/scvi/_settings.py set seed (+ what you added). better than the reuse of code.
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class SENADVAE(UnsupervisedTrainingMixin, BaseModelClass): |
There was a problem hiding this comment.
We usually use the VAE suffix to describe the module, not the model. It can be confusing.
How hard is is to change the model name to SENA and the module to SENAVAE
| nn.init.uniform_(self.bias, -bound, bound) | ||
|
|
||
|
|
||
| class SENAModule(BaseModuleClass): |
There was a problem hiding this comment.
Betten to have SENAVAE
|
|
||
| # Load and process Gene Ontology pathway annotations for biological constraints | ||
|
|
||
| # Load gene-to-GO mapping file containing pathway memberships |
There was a problem hiding this comment.
can we wrap this whole part of files pre-processing into a util function? It should also check for correctness of files and their structures.
seems that we would only want a function that, given those 3 file links, we will produce rel_dict, go_map and gos that the model will use, and I want to separate the way to get them from the real init of the model.
| perturbation_strings = adata.obs[perturbation_key].values | ||
|
|
||
| # Parse perturbation strings to extract unique intervention genes | ||
| for pert_str in perturbation_strings: |
There was a problem hiding this comment.
There is more pre-processing here, can it fits in a function elsewhere? We want to keep the setup_anndata function to do just that....
|
Hey @aperonalope , what is the status of the PR? Can we expect more commits to it? |
|
Hi Ori,
Thanks for checking in.
We’ve found a bug in the current implementation that affects the model’s
behavior, so we’re looking into its impact and working on a fix. Once
that’s resolved and things are stable, we’ll push additional commits to the
PR.
To avoid reviewing against a moving target, I’ll go through your comments
and confirm them after the fix is in and the updated version is available.
We agree that docs and a tutorial will be important for users, and we’ll
plan to tackle those once the core implementation is finalized.
Best,
Alvaro
El mié, 21 ene 2026 a las 8:30, Ori Kronfeld ***@***.***>)
escribió:
… *ori-kron-wis* left a comment (scverse/scvi-tools#3571)
<#3571 (comment)>
Hey @aperonalope <https://github.com/aperonalope> , what is the status of
the PR? Can we expect more commits to it?
Otherwise, could you please go over my comments here, review, and confirm
correctness?
We can release it without docs or a tutorial, but they are very important
for users to engage with the model.
—
Reply to this email directly, view it on GitHub
<#3571 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFJVIR4HEDNAOIF5ENSHM2L4H4TH7AVCNFSM6AAAAACJ4EXKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTONZWGU3TKOJRGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi Alvaro, any new updates? |
ori-kron-wis
added
on-merge: backport to 1.5.x
2 participants
No description provided.