Skip to content

Sort csr_matrix indices once on a combined row and column key - #3134

Open
antonwolfy wants to merge 1 commit into
masterfrom
sort-indices-composite-key
Open

antonwolfy wants to merge 1 commit into
masterfrom
sort-indices-composite-key

Conversation

@antonwolfy

Copy link
Copy Markdown
Contributor

dpnp.scipy.sparse.csr_matrix.sort_indices sorted 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 key row * 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.

  • The per-row offsets come from repeat(arange(0, nrows * ncols, ncols), row_lengths), so only one elementwise add runs before the sort. Computing row_ids * ncols + indices instead adds kernels that made small matrices slower than before on GPU (0.71-0.95x of master below about 3·10⁴ stored values).
  • The offsets are int64 even for int32 indices, so the key can't overflow. has_sorted_indices runs check_format first, which bounds the column indices to [0, ncols).
  • When nrows * ncols >= 2**63 the 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_indices alone (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.

nnz shape index GPU master, ms GPU PR, ms GPU speedup CPU master, ms CPU PR, ms CPU speedup
10^4 1250 x 1250 int32 3.8 3.7 1.04x 3.6 3.1 1.16x
10^4 1250 x 1250 int64 5.4 5.1 1.07x 3.7 3.1 1.20x
10^4 9 x 4096 int32 12.5 12.1 1.03x 3.5 3.1 1.14x
10^4 9 x 4096 int64 13.4 12.6 1.07x 3.8 3.0 1.25x
10^5 12500 x 12500 int32 19.4 15.2 1.28x 8.5 5.2 1.62x
10^5 12500 x 12500 int64 18.1 14.3 1.26x 8.4 5.1 1.63x
10^5 97 x 4096 int32 15.5 14.3 1.08x 7.9 4.9 1.63x
10^5 97 x 4096 int64 16.9 13.9 1.22x 7.8 4.7 1.66x
10^6 125000 x 125000 int32 51.7 23.0 2.25x 56.8 18.9 3.01x
10^6 125000 x 125000 int64 178 24.3 7.35x 71.6 23.6 3.03x
10^6 976 x 4096 int32 40.1 27.4 1.46x 70.4 23.8 2.96x
10^6 976 x 4096 int64 60.6 30.1 2.01x 68.5 24.0 2.85x
10^7 1250000 x 1250000 int32 1118 166 6.75x 855 160 5.33x
10^7 1250000 x 1250000 int64 1125 174 6.48x 916 166 5.52x
10^7 9765 x 9765 int32 325 167 1.95x 708 195 3.63x
10^7 9765 x 9765 int64 366 174 2.10x 719 193 3.73x

dpnp/tests/test_scipy_sparse_linalg.py passes (406 tests) on both of those devices.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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

@github-actions

Copy link
Copy Markdown
Contributor

View rendered docs @ https://intelpython.github.io/dpnp/pull/3134/index.html

@github-actions

Copy link
Copy Markdown
Contributor

Array API standard conformance tests for dpnp=0.22.0dev0=np2py314h8d9cdd5_4 ran successfully.
Passed: 1491
Failed: 0
Skipped: 6

@coveralls

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 82.638% (+0.002%) from 82.636% — sort-indices-composite-key into master

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants