You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(indexing): avoid full-grid allocations for sparse selections (#4356)
* fix(indexing): make sparse selections O(npoints) instead of O(nchunks)
CoordinateIndexer (general path and sorted-1D fast path) and
IntArrayDimIndexer built a dense per-chunk histogram via
np.bincount(..., minlength=nchunks) / np.zeros(nchunks) plus a
full-length cumsum. For arrays with very many chunks this allocates
memory proportional to the total chunk count regardless of how few
points are selected — a 3-point write to an array with 1.4e11 chunks
tried to allocate 1 TiB and raised MemoryError.
Replace the dense histogram with run boundaries computed directly on
the sorted raveled chunk ids (sorted_run_ends), storing a compressed
cumsum aligned with the occupied chunks, and index it positionally in
__iter__. Memory and time now scale with the number of selected
points.
Fixes#4174
Assisted-by: ClaudeCode:claude-fable-5
* refactor(indexing): keep dense indexer attributes as deprecated properties
The compressed per-occupied-chunk cumsum introduced for gh-4174 changed
the observable semantics of chunk_nitems_cumsum (and removed
chunk_nitems on IntArrayDimIndexer). Although zarr.core is documented
as private API, external code is known to introspect these indexers, so
be conservative: store the compressed offsets under a new name
(chunk_run_ends, aligned with chunk_rixs / dim_chunk_ixs) and restore
chunk_nitems / chunk_nitems_cumsum as properties that lazily rebuild
the original dense arrays, warning with ZarrDeprecationWarning. The
O(nchunks) cost is now only paid if someone actually accesses them —
which was the status quo before the fix.
Assisted-by: ClaudeCode:claude-fable-5
* fix(indexing): exact ceildiv everywhere and sparse boolean-axis selections
Review follow-ups for the sparse-selection change:
- Fix ceildiv itself rather than bypassing it at one call site. It went
through float division, so ceildiv(2**62 - 1, 1) returned 2**62; four
other callers (slice arithmetic, the regular-grid check, dask-style
chunk sizes) had the same latent error. Integers now divide exactly;
floats keep the ceil-of-quotient path. FixedDimension goes back to
calling it.
- BoolArrayDimIndexer still allocated a dense per-chunk count array and
ran a Python loop over every chunk, so an orthogonal boolean selection
on a finely chunked axis paid O(nchunks) in time and memory on top of
the mask. It now derives the occupied chunks and run ends from the
selected positions with sorted_run_ends, like the other two indexers,
and exposes the same deprecated dense properties. A boolean axis with
2**22 chunks goes from a multi-second loop to ~2 ms.
- Changelog: state precisely which selection kinds are covered, that
reading the deprecated properties rebuilds the dense array, and the
ceildiv correction.
Assisted-by: ClaudeCode:claude-fable-5-1
* fix(indexing): defer boolean-axis allocation changes
Retain sparse coordinate and integer indexing plus exact ceildiv, while restoring the existing boolean-axis implementation to avoid dense-mask regressions.
Assisted-by: Codex:GPT-6
* docs: describe current ceildiv behavior
Assisted-by: Codex:GPT-6
* fix(indexing): lazily cache legacy dense attributes
Preserve dense attribute identity, mutation behavior, and dataclass fields without deprecation warnings while keeping normal indexing sparse.
Assisted-by: Codex:GPT-6
* fix(indexing): use a dedicated integer ceiling division helper
Restore the original ceildiv behavior and exports, and use ceildiv_int for integer-only chunk and slice calculations without type-based dispatch.
Assisted-by: Codex:GPT-6
* refactor: remove unnecessary ceildiv re-exports
Assisted-by: Codex:GPT-6
* test: use Expect cases for integer ceiling division
Assisted-by: Codex:GPT-6
* test(indexing): restore the sparse projection property test
The test was added in a merge commit, which the linear rebase onto main dropped.
Assisted-by: ClaudeCode:claude-opus-5-5
* docs: number the changelog fragment after the merging PR
Towncrier renders fragment names as pull-request links, so the fragment
takes #4356 rather than issue #4174. The paragraph describing #4218 is
dropped: it shipped in the 3.4.0 release notes.
Assisted-by: ClaudeCode:claude-opus-5-5
0 commit comments