Skip to content

Commit d9d932f

Browse files
committed
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
1 parent a4b6643 commit d9d932f

6 files changed

Lines changed: 106 additions & 29 deletions

File tree

‎changes/4174.bugfix.md‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@ The intermediate `ChunksTuple` representation was removed in the process, and
99
size) with explicit edge-length sequences, matching what the normalizer and
1010
the rectilinear grid spec already accepted.
1111

12-
Coordinate, mask, and orthogonal integer-array selections no longer allocate memory
13-
proportional to the total number of chunks in the array. Previously a selection of a few
14-
points on an array with billions of chunks could raise `MemoryError`; indexing cost now
15-
scales with the number of selected points instead. The internal indexers now store
16-
per-occupied-chunk run offsets (`chunk_run_ends`); the dense `chunk_nitems` /
17-
`chunk_nitems_cumsum` attributes of `CoordinateIndexer` and `IntArrayDimIndexer` are
18-
deprecated but keep their original semantics.
12+
Coordinate selections (including `vindex` masks) and orthogonal selections with
13+
integer or boolean arrays no longer allocate memory proportional to the total number of
14+
chunks in the array, nor loop over every chunk in Python. Previously a selection of a few
15+
points on an array with billions of chunks could raise `MemoryError`, and a boolean array
16+
along a finely chunked axis paid a Python-level pass per chunk; indexing cost now scales
17+
with the number of selected points (plus, for a boolean array, the length of the array
18+
itself). The internal indexers now store per-occupied-chunk run offsets
19+
(`chunk_run_ends`); the dense `chunk_nitems` / `chunk_nitems_cumsum` attributes of
20+
`CoordinateIndexer`, `IntArrayDimIndexer` and `BoolArrayDimIndexer` are deprecated but
21+
keep their original semantics — note that reading them rebuilds the dense per-chunk array
22+
they used to hold, so on a huge array they still cost what they always did.
1923

20-
Uniform grid chunk counts now use exact integer arithmetic, so sparse selections
21-
near the end of dimensions larger than floating-point precision remain in bounds.
24+
`zarr.core.common.ceildiv` now divides integers exactly instead of through a float, so
25+
chunk counts of dimensions longer than `2**53` are correct; previously
26+
`ceildiv(2**62 - 1, 1)` rounded to `2**62`, which put sparse selections near the end of
27+
such a dimension out of bounds.

‎src/zarr/core/chunk_grids.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def __post_init__(self) -> None:
6262
if self.size == 0:
6363
n = 0
6464
else:
65-
n = -(-self.extent // self.size)
65+
n = ceildiv(self.extent, self.size)
6666
object.__setattr__(self, "nchunks", n)
6767
object.__setattr__(self, "ngridcells", n)
6868

‎src/zarr/core/common.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import asyncio
44
import math
5+
import numbers
56
import warnings
67
from collections.abc import Iterable, Mapping, Sequence
78
from enum import Enum
@@ -92,8 +93,21 @@ def product(tup: tuple[int, ...]) -> int:
9293

9394

9495
def ceildiv(a: float, b: float) -> int:
96+
"""Ceiling of ``a / b``.
97+
98+
Integer inputs are divided exactly: ``math.ceil(a / b)`` goes through a float and
99+
loses precision above ``2**53``, so e.g. ``ceildiv(2**62 - 1, 1)`` came out as
100+
``2**62``. Chunk counts of large arrays depend on this being exact.
101+
"""
95102
if a == 0:
96103
return 0
104+
# Widen to ``object`` for the check: mypy models ``int`` as a ``float`` and does not
105+
# know that it satisfies ``Integral``, so it would consider this branch unreachable.
106+
a_obj: object = a
107+
b_obj: object = b
108+
if isinstance(a_obj, numbers.Integral) and isinstance(b_obj, numbers.Integral):
109+
a_int, b_int = int(a_obj), int(b_obj)
110+
return -(-a_int // b_int)
97111
return math.ceil(a / b)
98112

99113

‎src/zarr/core/indexing.py‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -629,10 +629,10 @@ class BoolArrayDimIndexer:
629629
dim_grid: DimensionGrid
630630
nchunks: int
631631

632-
chunk_nitems: npt.NDArray[Any]
633-
chunk_nitems_cumsum: npt.NDArray[Any]
634632
nitems: int
635633
dim_chunk_ixs: npt.NDArray[np.intp]
634+
# end offset of each occupied chunk's run of selected items, aligned with dim_chunk_ixs
635+
chunk_run_ends: npt.NDArray[np.intp]
636636

637637
def __init__(
638638
self,
@@ -653,33 +653,51 @@ def __init__(
653653
g = dim_grid
654654
nchunks = g.nchunks
655655

656-
# precompute number of selected items for each chunk
657-
chunk_nitems = np.zeros(nchunks, dtype="i8")
658-
for dim_chunk_ix in range(nchunks):
659-
dim_offset = g.chunk_offset(dim_chunk_ix)
660-
chunk_len = g.data_size(dim_chunk_ix)
661-
chunk_nitems[dim_chunk_ix] = np.count_nonzero(
662-
dim_sel[dim_offset : dim_offset + chunk_len]
663-
)
664-
chunk_nitems_cumsum = np.cumsum(chunk_nitems)
665-
nitems = chunk_nitems_cumsum[-1]
666-
dim_chunk_ixs = np.nonzero(chunk_nitems)[0]
656+
# The selected positions are already in increasing order, so their chunk ids are
657+
# non-decreasing and the occupied chunks with their run ends follow in O(nitems).
658+
# The previous per-chunk count loop was O(nchunks) in both time and memory (gh-4174).
659+
sel_positions = np.nonzero(dim_sel)[0]
660+
dim_chunk_ixs, chunk_run_ends = sorted_run_ends(g.indices_to_chunks(sel_positions))
661+
nitems = int(sel_positions.size)
667662

668663
# store attributes
669664
object.__setattr__(self, "dim_sel", dim_sel)
670665
object.__setattr__(self, "dim_len", dim_len)
671666
object.__setattr__(self, "dim_grid", dim_grid)
672667
object.__setattr__(self, "nchunks", nchunks)
673-
object.__setattr__(self, "chunk_nitems", chunk_nitems)
674-
object.__setattr__(self, "chunk_nitems_cumsum", chunk_nitems_cumsum)
675668
object.__setattr__(self, "nitems", nitems)
676669
object.__setattr__(self, "dim_chunk_ixs", dim_chunk_ixs)
670+
object.__setattr__(self, "chunk_run_ends", chunk_run_ends)
671+
672+
@property
673+
@deprecated(
674+
"BoolArrayDimIndexer.chunk_nitems is deprecated: it materializes a dense array with "
675+
"one entry per chunk along the dimension. Use dim_chunk_ixs and chunk_run_ends, "
676+
"which cover only the occupied chunks.",
677+
category=ZarrDeprecationWarning,
678+
)
679+
def chunk_nitems(self) -> npt.NDArray[np.intp]:
680+
dense = np.zeros(self.nchunks, dtype=np.intp)
681+
dense[self.dim_chunk_ixs] = np.diff(self.chunk_run_ends, prepend=0)
682+
return dense
683+
684+
@property
685+
@deprecated(
686+
"BoolArrayDimIndexer.chunk_nitems_cumsum is deprecated: it materializes a dense array "
687+
"with one entry per chunk along the dimension. Use dim_chunk_ixs and chunk_run_ends, "
688+
"which cover only the occupied chunks.",
689+
category=ZarrDeprecationWarning,
690+
)
691+
def chunk_nitems_cumsum(self) -> npt.NDArray[np.intp]:
692+
dense = np.zeros(self.nchunks, dtype=np.intp)
693+
dense[self.dim_chunk_ixs] = np.diff(self.chunk_run_ends, prepend=0)
694+
return np.cumsum(dense)
677695

678696
def __iter__(self) -> Iterator[ChunkDimProjection]:
679697
g = self.dim_grid
680698

681699
# iterate over chunks with at least one item
682-
for dim_chunk_ix in self.dim_chunk_ixs:
700+
for i, dim_chunk_ix in enumerate(self.dim_chunk_ixs):
683701
# find region in chunk
684702
dim_offset = g.chunk_offset(dim_chunk_ix)
685703
chunk_len = g.data_size(dim_chunk_ix)
@@ -693,11 +711,11 @@ def __iter__(self) -> Iterator[ChunkDimProjection]:
693711
dim_chunk_sel = tmp
694712

695713
# find region in output
696-
if dim_chunk_ix == 0:
714+
if i == 0:
697715
start = 0
698716
else:
699-
start = self.chunk_nitems_cumsum[dim_chunk_ix - 1]
700-
stop = self.chunk_nitems_cumsum[dim_chunk_ix]
717+
start = self.chunk_run_ends[i - 1]
718+
stop = self.chunk_run_ends[i]
701719
dim_out_sel = slice(start, stop)
702720
is_complete_chunk = False # TODO
703721

‎tests/test_common.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from zarr.core.common import (
1111
ANY_ACCESS_MODE,
1212
AccessModeLiteral,
13+
ceildiv,
1314
concurrent_iter,
1415
parse_bool,
1516
parse_int,
@@ -24,6 +25,27 @@
2425
from typing import Any, Literal
2526

2627

28+
@pytest.mark.parametrize(
29+
("a", "b", "expected"),
30+
[
31+
(0, 3, 0),
32+
(7, 3, 3),
33+
(9, 3, 3),
34+
(2**62 - 1, 1, 2**62 - 1),
35+
(2**62 + 1, 2, 2**61 + 1),
36+
(2**60 + 3, 1, 2**60 + 3),
37+
(np.int64(2**62 - 1), np.int64(1), 2**62 - 1),
38+
(7.5, 2, 4),
39+
],
40+
)
41+
def test_ceildiv(a: float, b: float, expected: int) -> None:
42+
"""Integer inputs divide exactly, beyond the 2**53 range where float division rounds;
43+
float inputs keep the ceil-of-quotient semantics."""
44+
result = ceildiv(a, b)
45+
assert result == expected
46+
assert isinstance(result, int)
47+
48+
2749
@pytest.mark.parametrize("data", [(0, 0, 0, 0), (1, 3, 4, 5, 6), (2, 4)])
2850
def test_product(data: tuple[int, ...]) -> None:
2951
assert product(data) == np.prod(data)

‎tests/test_indexing.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from zarr.core.chunk_grids import ChunkGrid
2020
from zarr.core.indexing import (
2121
BasicSelection,
22+
BoolArrayDimIndexer,
2223
CoordinateIndexer,
2324
CoordinateSelection,
2425
IntArrayDimIndexer,
@@ -1358,6 +1359,15 @@ def test_deprecated_dense_indexer_attributes() -> None:
13581359
with pytest.warns(zarr.errors.ZarrDeprecationWarning):
13591360
assert_array_equal(dim_indexer.chunk_nitems_cumsum, np.cumsum(expected_nitems))
13601361

1362+
mask = np.zeros(20, dtype=bool)
1363+
mask[np.unique(coords)] = True
1364+
expected_mask_nitems = np.bincount(np.unique(coords) // 3, minlength=7)
1365+
bool_indexer = BoolArrayDimIndexer(mask, 20, dim_grid)
1366+
with pytest.warns(zarr.errors.ZarrDeprecationWarning):
1367+
assert_array_equal(bool_indexer.chunk_nitems, expected_mask_nitems)
1368+
with pytest.warns(zarr.errors.ZarrDeprecationWarning):
1369+
assert_array_equal(bool_indexer.chunk_nitems_cumsum, np.cumsum(expected_mask_nitems))
1370+
13611371

13621372
def test_sparse_selections_on_arrays_with_many_chunks(store: StorePath) -> None:
13631373
"""Coordinate and orthogonal selections must scale with the number of selected points,
@@ -1387,6 +1397,13 @@ def test_sparse_selections_on_arrays_with_many_chunks(store: StorePath) -> None:
13871397
vals = np.arange(1, len(coords) + 1, dtype="int32")
13881398
z.set_orthogonal_selection((np.array(coords), 0), vals)
13891399
assert_array_equal(z.get_orthogonal_selection((np.array(coords), 0)), vals)
1400+
# an orthogonal boolean selection along an axis with 2**22 chunks: the mask itself is
1401+
# O(dim_len), but the indexer must not add a per-chunk pass on top of it
1402+
mask = np.zeros(2**22, dtype=bool)
1403+
mask[[3, 5, 6, 2**22 - 1]] = True
1404+
vals = np.array([1, 2, 3, 4], dtype="int32")
1405+
z.set_orthogonal_selection((mask, 0), vals)
1406+
assert_array_equal(z.get_orthogonal_selection((mask, 0)), vals)
13901407

13911408

13921409
def test_coordinate_indexer_many_chunks() -> None:

0 commit comments

Comments
 (0)