model: SENAVAE #3571
model: SENAVAE #3571
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3571 +/- ##
===========================================
- Coverage 87.33% 70.80% -16.53%
===========================================
Files 224 229 +5
Lines 21029 22033 +1004
===========================================
- Hits 18365 15600 -2765
- Misses 2664 6433 +3769
🚀 New features to boost your workflow:
|
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....
2 participants
No description provided.