Add Seurat components #77
Conversation
| dense_temp <- as.matrix(norm_data) | ||
| norm_data <- as(dense_temp, "dgCMatrix") |
There was a problem hiding this comment.
Could this be one line to avoid making an extra copy?
There was a problem hiding this comment.
Thanks for the review!
Can do!
There was a problem hiding this comment.
Is intermediate dinsification strictly necessary? I'm worried that could cause a computational bottleneck for larger data
| cat("Generate UMAP embedding\n") | ||
| integrated <- RunUMAP( | ||
| integrated, | ||
| reduction = "pca", | ||
| dims = seq_len(par$dims), | ||
| verbose = FALSE | ||
| ) | ||
|
|
||
| cat("Extract embedding\n") | ||
| embedding <- Embeddings(integrated, reduction = "umap") |
There was a problem hiding this comment.
Why generate and store a UMAP instead of the integrated embedding?
There was a problem hiding this comment.
I agree with that, shouldn't it instead be:
| cat("Generate UMAP embedding\n") | |
| integrated <- RunUMAP( | |
| integrated, | |
| reduction = "pca", | |
| dims = seq_len(par$dims), | |
| verbose = FALSE | |
| ) | |
| cat("Extract embedding\n") | |
| embedding <- Embeddings(integrated, reduction = "umap") | |
| cat("Extract embedding\n") | |
| embedding <- Embeddings(integrated, reduction = "pca") |
Using UMAP for anything other than visualisation isn't common practice (and also not the default way of running the method)
There was a problem hiding this comment.
I think the integration might be stored as something like "integrated" but you would need to check the function calls.
I would suggest following this vignette https://satijalab.org/seurat/articles/seurat5_integration.
| anchors <- FindIntegrationAnchors( | ||
| object.list = seurat_list, | ||
| anchor.features = hvg_genes, | ||
| dims = seq_len(par$dims), | ||
| k.anchor = par$k_anchor, | ||
| k.filter = par$k_filter, | ||
| k.score = par$k_score, | ||
| verbose = FALSE | ||
| ) | ||
|
|
||
| # Integrate the data | ||
| integrated <- IntegrateData( | ||
| anchorset = anchors, | ||
| dims = seq_len(par$dims), | ||
| verbose = FALSE | ||
| ) |
There was a problem hiding this comment.
{Seurat} v5 has a new IntegrateLayers function which we should probably use instead https://satijalab.org/seurat/articles/seurat5_integration#perform-streamlined-one-line-integrative-analysis
There was a problem hiding this comment.
That's a good point. If the models are still the same, we should use the latest implementation for it.
| counts_data <- t(adata$layers[["counts"]]) | ||
| norm_data <- t(adata$layers[["normalized"]]) |
There was a problem hiding this comment.
are both matrices needed?
There was a problem hiding this comment.
Feature selection should ideally be standardised across methods
| cat("Generate UMAP embedding\n") | ||
| integrated <- RunUMAP( | ||
| integrated, | ||
| reduction = "pca", | ||
| dims = seq_len(par$dims), | ||
| verbose = FALSE | ||
| ) | ||
|
|
||
| cat("Extract embedding\n") | ||
| embedding <- Embeddings(integrated, reduction = "umap") |
There was a problem hiding this comment.
I agree with that, shouldn't it instead be:
| cat("Generate UMAP embedding\n") | |
| integrated <- RunUMAP( | |
| integrated, | |
| reduction = "pca", | |
| dims = seq_len(par$dims), | |
| verbose = FALSE | |
| ) | |
| cat("Extract embedding\n") | |
| embedding <- Embeddings(integrated, reduction = "umap") | |
| cat("Extract embedding\n") | |
| embedding <- Embeddings(integrated, reduction = "pca") |
Using UMAP for anything other than visualisation isn't common practice (and also not the default way of running the method)
| cat("Generate UMAP embedding\n") | ||
| integrated <- RunUMAP(integrated, reduction = "pca", dims = seq_len(par$dims), verbose = FALSE) | ||
|
|
||
| cat("Extract embedding\n") | ||
| embedding <- Embeddings(integrated, reduction = "umap") |
There was a problem hiding this comment.
Same as comment in CCA
| cat("Generate UMAP embedding\n") | |
| integrated <- RunUMAP(integrated, reduction = "pca", dims = seq_len(par$dims), verbose = FALSE) | |
| cat("Extract embedding\n") | |
| embedding <- Embeddings(integrated, reduction = "umap") | |
| cat("Extract embedding\n") | |
| embedding <- Embeddings(integrated, reduction = "pca") |
Describe your changes
Checklist before requesting a review
I have performed a self-review of my code
Check the correct box. Does this PR contain:
Proposed changes are described in the CHANGELOG.md
CI Tests succeed and look good!