feat: nicheVI release #3172
feat: nicheVI release #3172
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
please move this class into the _module.py file.
We cant have 2 base_module files in the codebase, very confusing.
|
|
||
| Examples | ||
| -------- | ||
| >>> loss_output = LossOutput( |
There was a problem hiding this comment.
you meant "NicheLossOutput" (its the same as original now), please make sure
| qkv = torch.cat([z_proj, cell_type_embedding], dim=2) | ||
| qkv = qkv.view(-1, qkv.size(2), qkv.size(3)) | ||
|
|
||
| p = self.transformer_encoder(qkv) |
There was a problem hiding this comment.
can the z.ndim be different than 2 or 3? in such case what will be the value of qkv?
| raise ValueError("Must provide either n_obs_minibatch or reconstruction_loss") | ||
|
|
||
| default = 0 * self.loss | ||
| if self.reconstruction_loss is None: |
There was a problem hiding this comment.
seems to me those next lines can be simply re-written in a for loop of different attributes, all is going under same thing (but I guess we need to fix also original class)
There was a problem hiding this comment.
a tutorial and model description doc are missing
There was a problem hiding this comment.
give better name for file
| plt.show() | ||
|
|
||
|
|
||
| # def _fdr_de_prediction(posterior_probas: pd.Series, fdr: float = 0.05) -> pd.Series: |
There was a problem hiding this comment.
what is under comment please remove. we do not merge such things to main
There was a problem hiding this comment.
this is also already exists for the main use of DE. why do we need to have this duplications?
Can't it be inherited from updated DE , init and adjusted locally with those parameters? will save alot of code.
| nichevae.predict_niche_activation() # specific to nicheSCVI | ||
|
|
||
|
|
||
| def test_nichevi_differential(adata): |
There was a problem hiding this comment.
except for the fact that it is actually running, dont you also want to verify the pytest to give us good results?
| early_stopping=True, | ||
| check_val_every_n_epoch=1, | ||
| accelerator="cpu", | ||
| ) |
There was a problem hiding this comment.
I would add to check if model is trained and several down stream analysis function you should use
No description provided.