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

Write DelayedArray to file on the R side. #35

Merged
merged 11 commits into from
Feb 18, 2021
Merged

Conversation

LTLA
Copy link
Contributor

@LTLA LTLA commented Jan 27, 2021

Closes #32.

library(zellkonverter)
library(scRNAseq)
sce <- ZeiselBrainData()
counts(sce) <- DelayedArray(counts(sce))
 
temp <- tempfile(fileext = ".h5ad")
writeH5AD(sce, temp)

Some points:

  • Need to test that it works with both DA in the first and/or later assays.
  • Need to test that the result is still readable by readH5AD.

Known to-dos:

  • The code does not respect is_sparse() being TRUE. A little bit of work is required to achieve a one-pass writer that does not rely on knowing the total number of non-zero entries.

R/write.R Outdated Show resolved Hide resolved
@LTLA
Copy link
Contributor Author

LTLA commented Jan 29, 2021

Latest commit solves the to-do above, but needs a lot of testing.

Briefly, we set up extensible HDF5 datasets and then iterate across row-wise chunks of the sparse DA. In each chunk, we extract the non-zero values, convert them to compressed sparse row format, and append them to the existing HDF5 dataset via h5set_extent (to make the existing dataset larger) and h5writeDataset to write the new values in the expanded space. This approach respects the memory constraints of block processing, by ensuring that we do not load all non-zero values into memory; while retaining a single-pass approach for rapid processing, by avoiding the need to know the total number of non-zeroes.

The code here makes a number of assumptions:

  • I used values, indptrs and index for the values, column indices and pointers, respectively. Can't remember what H5AD actually calls them. Writing this now, I suspect I mixed up index and indptrs.
  • Column indices are assumed to be zero-based.
  • Stored the pointers as 64-bit unsigned integers as the number of non-zeros can exceed ~3 billion for large datasets.

@lazappi
Copy link
Member

lazappi commented Jan 30, 2021

Added a couple of tests and fixed how the loop for rewriting matrices handled paths.

@LTLA
Copy link
Contributor Author

LTLA commented Jan 30, 2021

Hit a roadblock in the form of grimbough/rhdf5#79; the AnnData reader doesn't like the fixed-width byte strings that rhdf5 emits.

For the time being, I suggest we just add a clause to the initial check for DAs where we do not skip a DA if it is_sparse() is TRUE. This implies that it will be realized into memory in SCE2AnnData - hopefully this is tolerated on the user machine. We can switch back to the current block processing behavior once the rhdf5 issue is resolved.

@LTLA
Copy link
Contributor Author

LTLA commented Feb 12, 2021

@lazappi were you going to work on this?

@lazappi
Copy link
Member

lazappi commented Feb 12, 2021

Sorry I've lost track of it a bit. Is it just the is_sparse() check that we need to add?

@LTLA
Copy link
Contributor Author

LTLA commented Feb 12, 2021

Yes, I think line 60 in my PR should only skip the assay if it's a DA and !is_sparse(). Sparse DAs should be coerced to in-memory dgCMatrix objects for the time being.

@lazappi
Copy link
Member

lazappi commented Feb 16, 2021

Ok, I've added that check. The code for writing sparse DelayedArrays can't be reached by anything now which made the test coverage drop but that's fine and better to have it there for when the rhdf5 issue is fixed. Anything else to add?

@LTLA
Copy link
Contributor Author

LTLA commented Feb 16, 2021

Think that's it, just slap on some # nocov start and ends around it for the time being, I guess.

@lazappi lazappi merged commit 62ff57f into theislab:master Feb 18, 2021
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.

Skip DelayedArray matrices in SCE2AnnData()
2 participants