Skip to content

perf(zarr-indexing): skip the sort in _last_occurrences for sorted indices - #4603

Merged
d-v-b merged 4 commits into
zarr-developers:mainfrom
d-v-b:claude/zarr-indexing-last-occurrences-fastpath-a3bb59
Nov 6, 2026
Merged

d-v-b merged 4 commits into
zarr-developers:mainfrom
d-v-b:claude/zarr-indexing-last-occurrences-fastpath-a3bb59

Conversation

@d-v-b

@d-v-b d-v-b commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

performance win for pre-sorted coordinate indices.

🤖 AI text below 🤖

Writes whose indices land on a chunk in sorted order no longer sort them to drop repeated indices, so deduplication is linear in the number of points; writes without repeats also skip a gather copy. Unsorted indices behave as before.

_last_occurrences now checks in O(n) whether the flat positions are non-decreasing, the usual case since a chunk's points arrive in C order. If they are, repeats are adjacent and the last of each run is kept via np.flatnonzero(np.append(flat[1:] != flat[:-1], True)). If there are no repeats, or fewer than two positions, it returns slice(None), so _scatter_numpy and _scatter_planned index without copying. Unsorted input still goes through np.unique. Callers only use the result to pick matching rows of coordinates and values, so the order of the kept positions doesn't matter. The sorted-input check is borrowed from the last_occurrences helper that Bhaskar Gurram (bhaskargurram-ai) wrote in #4601; credit for the idea goes to them.

A new parametrized test, test_last_occurrences_matches_sequential_assignment, checks empty, single, sorted, sorted-with-repeats, unsorted-with-repeats and reversed input against sequential NumPy assignment.

Timing for _last_occurrences on 5M intp positions (NumPy 2.5.1, Python 3.13, macOS arm64, best of 5):

input before after speedup
sorted, no repeats 12.9 ms 1.2 ms 11.0x
sorted, repeats 183.3 ms 3.1 ms 59.6x
unsorted, repeats 766.7 ms 765.2 ms 1.0x

The zarr-indexing suite passes on Python 3.13 (1745 passed), and just typecheck and just lint are clean. On Python 3.15, four tests fail on main too, because they match the __index__ error message that 3.15 changed.

🤖 Generated with Claude Code

Author attestation

  • [ x ] I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.md
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

d-v-b and others added 3 commits November 6, 2026 15:19
…dices

When the flat in-hull positions are non-decreasing (the usual C-order
case), repeats are adjacent and the last occurrence of each can be found
in O(n) without np.unique. When there are no repeats at all, return a
full slice so callers index without a gather copy.

Assisted-by: ClaudeCode:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assisted-by: ClaudeCode:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added needs release notes Automatically applied to PRs which haven't added release notes zarr-indexing Specific to the zarr-indexing sub-package labels Nov 6, 2026
@d-v-b
d-v-b marked this pull request as ready for review November 6, 2026 14:29
…s#4603

Assisted-by: ClaudeCode:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@d-v-b
d-v-b merged commit 9b82146 into zarr-developers:main Nov 6, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs release notes Automatically applied to PRs which haven't added release notes zarr-indexing Specific to the zarr-indexing sub-package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant