Repository navigation
benchmarks: narrow a store worth narrowing, and report sizes - #44
Conversation
The dtype case cast the main fixture to float32, which it already was, so it measured a rewrite that changed nothing: 291 MB in, 291 MB out. recast-narrow now starts from a copy of the main input with X stored as float64 values and int64 indices, narrows it to float32/int32, and its table reads input -> output with the change; convert-layout reports the same, since 0.7.0 fixed a --layout that tripled the file. Two wide variants, because compression decides the answer: integer counts in float64 are mostly zero bytes, so narrowing an uncompressed X halves the file while an lzf one shrinks by a few percent. recast-narrow uses the uncompressed one (write_h5ad's default), recast-narrow-lzf the other. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23522bc591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "X.indices = X.indices.astype('int32')\n" | ||
| "X.indptr = X.indptr.astype('int32')\n" | ||
| "obj.X = X\n" | ||
| "obj.write_h5ad(OUT, compression=None)\n" |
There was a problem hiding this comment.
Preserve compression for non-X elements in the baseline
In the recast-narrow case, the wide fixture has only X made uncompressed; _widen copies obs, var, obsm, and other elements from the original lzf-compressed fixture unchanged. The CLI likewise preserves those elements' compression, but write_h5ad(..., compression=None) rewrites the entire baseline output without compression. Consequently, the reported runtime and input-to-output size compare an X recast plus unrelated decompression against a pure recast. Preserve the non-X datasets' filters in the baseline, or make the whole input uncompressed for both contenders.
Useful? React with 👍 / 👎.
The recast benchmark measured nothing.
convert-dtypecast the main fixture to float32, but the fixture was already float32 with int32 indices, so 291 MB went in and 291 MB came out.What changed
recast-narrow(headline) replacesconvert-dtype. It starts from a copy of the main input with X stored as float64 values and int64 indices/indptr, and narrows it to float32/int32. That's the situation issue Feature request. Change dtype of X #13 describes.recast-narrow-lzf: the same narrowing on an lzf-compressed X (see below for why there are two).input → output (±%). That includesconvert-layout, where 0.7.0 fixed a--layoutthat tripled the file.Why two variants
Whether narrowing saves disk space depends on compression. Integer counts stored as float64 are mostly zero bytes, which a compressor strips anyway. On a 20M-nonzero matrix:
Uncompressed is
write_h5ad's default, so it's the headline case. The lzf case keeps the small saving visible. On a compressed store, narrowing pays off in memory once the matrix is loaded, not on disk.Local
ci-tier run (50,000 × 20,000, warm cache, one repeat)recast-narrowrecast-narrow-lzfconvert-layout(streaming)Two new harness tests cover this: one checks that the wide fixtures hold the same matrix in wider dtypes, and one checks the size column's rendering.
🤖 Generated with Claude Code