Conversation
src/cellflow/training/_callbacks.py
Outdated
| for cond in conditions_adata & conditions_pred: | ||
| true_counts[name][cond] = self.validation_adata[name][ | ||
| self.validation_adata[name].obs[self.condition_id_key] == cond | ||
| ].X.toarray() |
There was a problem hiding this comment.
Could we potentially go OOM here when using real data? I was not sure @MUCDK
There was a problem hiding this comment.
should be fine, because we first subset the adata before densifying.
two things here:
- provide the option to extract any adata.X or any adata.layers[key], similar how we e.g. do it in moscot
- bear in mind that adata.X can be both sparse and dense
|
@MUCDK Your suggestions have been implemented, pinged since i cant request a review right now. |
src/cellflow/model/_cellflow.py
Outdated
| self._dataloader: TrainSampler | None = None | ||
| self._trainer: CellFlowTrainer | None = None | ||
| self._validation_data: dict[str, ValidationData] = {} | ||
| self._validation_adata: dict[str, ad.Anndata] = {} |
There was a problem hiding this comment.
can we make these attributes of the callbacks directly?
src/cellflow/training/_callbacks.py
Outdated
| self.layers = layers | ||
| self.log_prefix = log_prefix | ||
|
|
||
| def add_validation_adata( |
There was a problem hiding this comment.
this can be part of __init__, can't it?
src/cellflow/training/_trainer.py
Outdated
| for callback in callbacks: | ||
| if isinstance(callback, PCADecodedMetrics2): | ||
| callback.add_validation_adata(self.validation_adata) |
There was a problem hiding this comment.
I don't think this would be needed if validation_adata was an attribute of the callback, would it?
MUCDK
left a comment
MUCDK
left a comment
There was a problem hiding this comment.
Let's try to make the validation_adata an attribute of the callback, as outlined in the comments.
2 participants
Adressing #220.