Repository navigation
Sort csr_matrix indices once on a combined row and column key - #3134
Open
antonwolfy wants to merge 1 commit into
Open
antonwolfy wants to merge 1 commit into
antonwolfy wants to merge 1 commit into
Conversation
sort_indices sorted entries by (row, column) with two stable argsorts. Sort once on the int64 key row * ncols + col instead, built from repeated per-row offsets, and keep the two-pass sort when the key would overflow int64.
antonwolfy
requested review from
ndgrigorian and
vlad-perevezentsev
as code owners
October 30, 2026 15:24
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the CSR index-sorting optimization.
No open issues 🎉
What changed in this PR
This PR optimizes CSR index sorting in dpnp.scipy.sparse by replacing two stable sorts with one stable sort on a combined row-and-column key when it fits in int64.
Changes:
- Uses int64 row offsets and preserves the two-pass fallback for shapes at or above the int64 key limit.
- Adds regression tests for int32 key overflow and shapes around the int64 limit.
- Documents the performance improvement in the changelog.
| File | Description |
|---|---|
dpnp/tests/test_scipy_sparse_linalg.py |
Adds overflow and large-shape sorting regression tests. |
dpnp/scipy/sparse/_csr.py |
Implements combined-key sorting with an overflow-safe fallback. |
CHANGELOG.md |
Records the CSR index-sorting performance improvement. |
Review effort: Balanced
Contributor
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/3134/index.html |
Contributor
|
Array API standard conformance tests for dpnp=0.22.0dev0=np2py314h8d9cdd5_4 ran successfully. |
Collaborator
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.
dpnp.scipy.sparse.csr_matrix.sort_indicessorted the stored entries by (row, column) with two stable argsorts: one on the column indices, then one on the permuted row ids. This PR sorts once on the int64 keyrow * ncols + col. CSR rows are already in order, so the key is nearly sorted, and one merge sort of it is much cheaper than two sorts of fully permuted data.repeat(arange(0, nrows * ncols, ncols), row_lengths), so only one elementwise add runs before the sort. Computingrow_ids * ncols + indicesinstead adds kernels that made small matrices slower than before on GPU (0.71-0.95x of master below about 3·10⁴ stored values).has_sorted_indicesrunscheck_formatfirst, which bounds the column indices to[0, ncols).nrows * ncols >= 2**63the key doesn't fit int64, and the previous two-pass sort is kept. Its row ids are now int64 too.kind="stable"(merge sort for int64) stays: radix sort on the int64 key was slower on both devices measured.Two new tests: an int32-index matrix whose key overflows int32 (shape 3001 x 10⁶), and shapes just below, at and above the int64 key limit, the last of which takes the fallback. They fail when the offsets use the index dtype or when the fallback is removed. Both allocate only a few elements whatever the shape, so they aren't marked
slow.Performance
sort_indicesalone (construction excluded), master vs this PR loaded in the same process, median time, on a Panther Lake H machine: the Xe3 iGPU on Level Zero and the Core Ultra 7 366H on OpenCL. Each matrix has 8 or about 1024 stored values per row, with columns shuffled within each row; results are identical to master in every case.dpnp/tests/test_scipy_sparse_linalg.pypasses (406 tests) on both of those devices.