Repository navigation
Pass depends to the _place call in dpnp.tensor.place - #3164
Merged
Merged
Conversation
place() casts vals to the array's data type with dpt.astype, which is asynchronous, and then called ti._place without depends, so the kernel could read the cast buffer before the cast finished.
ndgrigorian
requested review from
antonwolfy and
vlad-perevezentsev
as code owners
November 5, 2026 07:27
Contributor
|
Array API standard conformance tests for dpnp=0.22.0dev0=np2py314h8d9cdd5_32 ran successfully. |
Contributor
|
View rendered docs @ https://intelpython.github.io/dpnp/index.html |
Collaborator
|
coverage: 84.396%. remained the same — fix-place-missing-depends into master |
There was a problem hiding this comment.
🟡 Changes recommended
A deterministic regression test is still needed to verify that _place receives the cast event in depends, since the existing value-only assertion can pass when that dependency is missing.
1 open issue
What changed in this PR
This PR fixes asynchronous ordering in dpnp.tensor.place, also used by dpnp.place, so writes wait for the values’ dtype conversion to finish.
Changes:
- Passes the queue’s submitted events to
_placeto prevent incorrect values on out-of-order queues. - Adds a changelog entry describing the fix.
| File | Description |
|---|---|
dpnp/tensor/_indexing_functions.py |
Forwards pending event dependencies to _place. |
CHANGELOG.md |
Documents the ordering fix for both public place functions. |
Review effort: Balanced
ndgrigorian
changed the title
dpnp.tensor.place
ndgrigorian
changed the title
dpnp.tensor.place_place call in dpnp.tensor.place
github-actions Bot
added a commit
that referenced
this pull request
Nov 5, 2026
`dpnp.tensor.place` casts `vals` to the array's data type with `dpt.astype`, which is asynchronous, and then called `ti._place` without `depends`. The kernel could start before the cast finished and write values from an incomplete buffer into the array. `dpnp.place` goes through the same path. The `mask_positions` call earlier in the function is synchronous, so only the cast was unordered. This PR passes `depends=_manager.submitted_events` to `ti._place`. The race is timing-dependent, so no test is added. On an out-of-order queue of an Intel CPU (OpenCL), `place` of an `int32` `vals` into a `float32` array, with the cast held back behind a few large copies, gave wrong values in 9 of 10 runs before this change and in 0 of 10 after. On a Level Zero iGPU it did not reproduce either way. 1439b19
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.tensor.placecastsvalsto the array's data type withdpt.astype, which is asynchronous, and then calledti._placewithoutdepends. The kernel could start before the cast finished and write values from an incomplete buffer into the array.dpnp.placegoes through the same path.The
mask_positionscall earlier in the function is synchronous, so only the cast was unordered. This PR passesdepends=_manager.submitted_eventstoti._place.The race is timing-dependent, so no test is added. On an out-of-order queue of an Intel CPU (OpenCL),
placeof anint32valsinto afloat32array, with the cast held back behind a few large copies, gave wrong values in 9 of 10 runs before this change and in 0 of 10 after. On a Level Zero iGPU it did not reproduce either way.