Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repair and optimize PsiNorm() #108

Merged
merged 1 commit into from
Feb 19, 2025
Merged

Repair and optimize PsiNorm() #108

merged 1 commit into from
Feb 19, 2025

Conversation

hpages
Copy link
Contributor

@hpages hpages commented Feb 19, 2025

Hi,

A recent change in DelayedArray (in version 0.33.5) broke PsiNorm() on a sparse DelayedMatrix object and on a SE/SCE object with counts represented as a sparse DelayedMatrix object. See https://bioconductor.org/checkResults/3.21/bioc-LATEST/scone/. Sorry for that!

This commit fixes the problem and also introduces two small improvements to PsiNorm():

  • makes PsiNorm() work on a SparseMatrix object;
  • makes PsiNorm() slightly faster by reducing the number of matrix transpositions from 4 to 2.

For example:

library(scone)

library(DelayedArray)
x <- DelayedArray(poissonSparseMatrix(10, 8))
# Broken in scone 1.31.0 (due to recent change in DelayedArray) but works again in scone 1.31.1
PsiNorm(x)

library(TENxPBMCData)
sce <- TENxPBMCData("pbmc4k")
# Also broken in scone 1.31.0 (due to recent change in DelayedArray) but works again in scone 1.31.1
PsiNorm(sce)

Other improvements:

x0 <- poissonSparseMatrix(5000, 77000)
a0 <- as.matrix(x0)

PsiNorm(a0)  # 50% speedup compared to scone 1.30.0

PsiNorm(x0)  # NEW (not supported in scone 1.30.0), 10x faster than 'PsiNorm(a0)'

sce <- SingleCellExperiment(list(counts=x0))
PsiNorm(sce)  # NEW (not supported in scone 1.30.0)

Best,
H.

A recent change in DelayedArray (in version 0.33.5) broke PsiNorm()
on a sparse DelayedMatrix object and on a SE/SCE object with counts
represented as a sparse DelayedMatrix object.
This commit repairs that and introduces two small improvements to
PsiNorm():
- makes PsiNorm() work on a SparseMatrix object;
- makes PsiNorm() slightly faster by reducing the number of matrix
  transpositions from 4 to 2.
@drisso
Copy link
Contributor

drisso commented Feb 19, 2025

Thank you @hpages

@drisso drisso merged commit 120e114 into YosefLab:master Feb 19, 2025
0 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants