Repository navigation
On-disk and DelayedArray layers: join, stitch, save, reload (replaces four PRs) - #320
Open
BenjaminDEMAILLE wants to merge 13 commits into
Open
BenjaminDEMAILLE wants to merge 13 commits into
BenjaminDEMAILLE wants to merge 13 commits into
Conversation
Mirror StitchMatrix.IterableMatrix for DelayedMatrix (DelayedArray/HDF5Array) layers so multi-layer assays backed by on-disk/64-bit DelayedMatrix can be joined (JoinLayers) and scaled. Uses DelayedArray::arbind/acbind to avoid base/S4Vectors cbind mis-dispatch. DelayedArray is already in Suggests. Companion to satijalab/seurat#9798. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve DESCRIPTION version collision (main bumped to 5.4.0.9001 via satijalab#294); set to 5.4.0.9002 so the StitchMatrix.DelayedMatrix dev snapshot is distinct from main's. Move the StitchMatrix.DelayedMatrix NEWS entry from the released 5.4.0 section up to # Unreleased alongside satijalab#294. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SaveSeuratRds() records where each on-disk layer lives and LoadSeuratRds() reopens it from that record, but the record was only ever interpreted from the current working directory. Both ways of saving therefore broke as soon as the object was shared or reorganized: - relative = TRUE writes paths relative to the rds, so loading from any other working directory found nothing; - relative = FALSE writes absolute paths, so moving or renaming the folder holding the rds and its stores found nothing. In both cases LoadSeuratRds() warned "Cannot find any of the layer files specified" and returned an object with no layers at all. Resolve each cached path against the directory holding the rds file before checking it: try the recorded path as-is, then relative to that directory, then by name within it. Objects that have not moved keep loading through the first branch, so existing behaviour is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The resolver handles comma-separated path entries, since some on-disk classes record more than one file per layer, but nothing exercised that branch. Add unit tests for it, plus the cases that must not be disturbed: a component that already resolves is returned byte-identical, an unresolvable one is returned unchanged rather than rewritten, and NA, empty and dir = NULL inputs pass straight through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merging layers whose combined non-zero count passes 2^31 fails with Error: vector::reserve reported by several users against JoinLayers on objects assembled from many studies (satijalab/seurat#10009). RowMergeMatricesList accumulates the per-layer counts into a signed int: int num_nZero = 0; num_nZero += mat_vec[i].nonZeros(); tripletList.reserve(num_nZero); Past 2^31 that wraps negative, and reserve() converts the negative value to a huge size_t and throws std::length_error. The message is "vector" on libc++ and "vector::reserve" on libstdc++, which is what users see. Note this only affects layers with differing feature names. When every layer shares the same rownames, RowMergeSparseMatrices takes the cbind path instead, where Matrix raises a clear error of its own. Merging studies with different gene sets is exactly the reported case. Accumulate in int64_t. A result too large for a dgCMatrix to index can still not be built, but that is now stated plainly, with the count, the limit and the options, rather than surfacing as a standard library exception. Verified on two layers of 1.1e9 non-zeros each (2.2e9 combined, wrapping to -2094967296): before, "Error: vector"; after, the explanatory error. That case needs ~60 GB, so it is not in the test suite; the added tests cover the normal merge paths the change touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard added in the previous commit told users to keep their layers split or move to BPCells. That reads as "give up", and it is incomplete: StitchMatrix dispatches on layer class, and only the dgCMatrix method goes through this C++ routine. Layers held as a DelayedMatrix are stitched by StitchMatrix.DelayedMatrix, which binds lazily and is not bound by the 2^31 index limit at all. Verified by running the same 2.2e9 non-zero case through DelayedMatrix layers: JoinLayers succeeds, giving a 3000 x 1100000 DelayedMatrix, with zeros where a feature is absent from a layer and the original values where it is present. Name that route in the message, so someone who hits the limit is told what actually works rather than only what to avoid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading a layer costs time proportional to the number of cells, while reaching into the slot directly is free: 2000 x 5000 LayerData 0.0068 s @layers 0.0000 s 2000 x 30000 LayerData 0.0120 s @layers 0.0000 s Profiling puts 92% of that in ..subscript.2ary. LayerData() resolves the requested features and cells to indices, then always subsets: slot(object, 'layers')[[layer]][features, cells, drop = FALSE] Asking for a layer whole, which is what nearly every call does, produces indices covering every row and column in order, so the subset copies the entire matrix to return it unchanged. Extrapolated, the 1.3 s the report measures corresponds to a few million cells. Skip the subset when the indices already select everything in order. 2000 x 5000 0.0068 s -> 0.0001 s 2000 x 30000 0.0120 s -> 0.0002 s and the cost no longer grows with cell count. The whole-layer path now returns the stored matrix rather than a copy. R's copy-on-write keeps that safe, and there is a test asserting that modifying the returned matrix does not reach back into the assay. Seurat's suite is unchanged: 2 failures, 14 errors, 294 passes, 1 warning before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… into group/so-ondisk
… into group/so-ondisk # Conflicts: # NEWS.md
A layer joined from several on-disk matrices records their paths as one comma-separated entry, which is what .FilePath() returns for it. SaveSeuratRds() passed that entry to .FileMove() whole: > obj[["RNA"]] <- JoinLayers(obj[["RNA"]]) > SaveSeuratRds(obj, file) Error: Can't find path: /tmp/store1,/tmp/store2 so a BPCells object could be joined or saved, but not both. Split the entry, move each store, and record them the same way. LoadSeuratRds() already splits on the comma when resolving. Verified end to end: an object with a joined layer saves, loads back with its layer intact, and the values match the matrix it was built from. Fixes satijalab#283. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 1, 2026
…to group/so-ondisk # Conflicts: # NEWS.md
BenjaminDEMAILLE
changed the title
This was referenced Sep 1, 2026
BenjaminDEMAILLE
marked this pull request as ready for review
September 1, 2026 06:38
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft, and grouped on purpose. This replaces four separate PRs, all about on-disk layers and large objects, plus one fix made since. I have closed the ones it supersedes: #295, #297, #298, #310. The individual commits are preserved.
Marked draft so it does not sit in your review queue until you say you want it.
The four belong together: they are one round trip. #298 is what breaks when joining, #283 what breaks when saving the result, #297 what breaks when reading it back somewhere else, and #310 what makes reading a layer affordable in the first place.
JoinLayersoverflowError: vector::reservefrom a 32-bit count on large objects (#298)SaveSeuratRdson joined layers.FileMove()whole:Can't find path: /tmp/store1,/tmp/store2. So a BPCells object could be joined, or saved, but not both (#283)LoadSeuratRdsportabilityrelative = TRUEproducing a half-relative path)LayerDataStitchMatrix.DelayedMatrixDelayedArrayhad no stitching method, so a split object holding them could not be rejoined; needed by satijalab/seurat#10493Verification
Each commit carries a test that fails without its fix. The joined-store fix is verified end to end: an object with a joined layer saves, loads back with its layer intact, and the values match the matrix it was built from. Full suite: 226 passing, zero failures.
R CMD check: Status OK.🤖 Generated with Claude Code