Skip to content

Fix row corruption in batched multi-level cumulative scan - #3063

Merged
antonwolfy merged 5 commits into
masterfrom
fix-cumulative-batched-multilevel-scan
Sep 9, 2026
Merged

Fix row corruption in batched multi-level cumulative scan#3063
antonwolfy merged 5 commits into
masterfrom
fix-cumulative-batched-multilevel-scan

Conversation

@antonwolfy

@antonwolfy antonwolfy commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

dpnp.cumsum, dpnp.cumprod, and their nan/cumulative_* variants (including dpnp.tensor.cumulative_sum/cumulative_prod) could silently return incorrect results when accumulating along an axis of a multi-row array, with no error raised. The corruption is triggered when the accumulation axis is long enough to require three or more reduction levels in the internal block-scan (axis length greater than chunk_size ** 2, where chunk_size = wg_size * n_wi, i.e. roughly a million elements on GPU and about four million on CPU) and the array has more than one batch row. The elementwise total is conserved but redistributed across rows.

Root cause

In the batched scan driver inclusive_scan_iter (accumulators.hpp), the intermediate block-scan fixup update_local_chunks passed a NoOpIndexer for the per-row (iter) offset into the src buffer. The intermediate block-scan buffers, however, are laid out strided per row with a stride of src_size. The local_scans read already used the correct iter_gid * local_stride offset, but the src write offset was unstrided, so rows overwrote each other. This cancels out only when iter_nelems == 1 (a single row) or when fewer than three levels are needed, which is why the bug went unnoticed.

Fix

Give update_local_chunks a Strided1DIndexer{iter_nelems, src_size} for the row offset instead of the NoOpIndexer (the within-row out_indexer stays NoOp, since the level buffers are contiguous within a row).

  • 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?

The intermediate block-scan update in inclusive_scan_iter used an
unstrided (NoOp) row offset into the strided per-row scan buffers, so
cumulative_sum/cumulative_prod along an axis returned wrong results
whenever the scan needed >=3 reduction levels (axis > chunk_size**2)
with more than one batch row. Use a Strided1DIndexer with the per-row
stride instead.

Add a regression test exercising the >=3-level batched scan path.
@antonwolfy antonwolfy added this to the 0.21.0 release milestone Sep 8, 2026
@antonwolfy antonwolfy self-assigned this Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Array API standard conformance tests for dpnp=0.21.0dev8=py314ha0e2e8e_8 ran successfully.
Passed: 1376
Failed: 0
Skipped: 6

@coveralls

coveralls commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 78.495%. remained the same — fix-cumulative-batched-multilevel-scan into master

@antonwolfy
antonwolfy marked this pull request as ready for review September 8, 2026 15:10
Comment thread dpnp/tests/tensor/test_tensor_accumulation.py

@ndgrigorian ndgrigorian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM other than minor nit

@ndgrigorian
ndgrigorian self-requested a review September 8, 2026 15:41
@antonwolfy
antonwolfy merged commit d054838 into master Sep 9, 2026
102 of 109 checks passed
@antonwolfy
antonwolfy deleted the fix-cumulative-batched-multilevel-scan branch September 9, 2026 11:32
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