Skip to content

Fix #1169: correct NumPy skip conditions for DLPack host writes#2238

Merged
seberg merged 4 commits into
NVIDIA:mainfrom
ArsalanShakil:fix/1169-numpy-dlpack-skip-conditions
Jul 19, 2026
Merged

Fix #1169: correct NumPy skip conditions for DLPack host writes#2238
seberg merged 4 commits into
NVIDIA:mainfrom
ArsalanShakil:fix/1169-numpy-dlpack-skip-conditions

Conversation

@ArsalanShakil

@ArsalanShakil ArsalanShakil commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

What

Audit and fix the NumPy version skip/guard conditions across cuda_core, per #1169.

Why

Writing into a host-accessible array obtained via np.from_dlpack(...) requires NumPy 2.2.5+: earlier versions return a read-only array (numpy GH #28632), so the write raises ValueError: assignment destination is read-only. Several tests and examples guarded these writes at NumPy 2.1.0, which only covers basic DLPack support. On NumPy 2.1.0–2.2.4 those tests/examples would error instead of skip — exactly the situation #1169 (and parent epic #1168) flagged.

Changes

Classification rule applied to every NumPy guard: writes into a np.from_dlpack host array → require 2.2.5; read-only DLPack usage → 2.1 is sufficient.

Bumped to 2.2.5 (with a consistent reason="need numpy 2.2.5+ (numpy GH #28632)", matching the already-correct guard in test_launcher.py):

  • Examples: memory_ops.py, graph_update.py, memory_pool_resources.py (all write to pinned/managed DLPack arrays)
  • Tests: test_launcher.py (2 sites), test_graph_builder.py (2 sites), test_graph_update.py (2), test_device_launch.py (2), test_graph_definition_mutation.py (2), test_graph_builder_conditional.py (4)

Additional cleanups:

  • examples/memory_ops.py used a plain string comparison np.__version__ < "2.1.0", which is unreliable for multi-digit versions (e.g. "2.9" > "2.10" lexicographically). Replaced with np.lib.NumpyVersion, consistent with the other examples.
  • test_graph_builder.py had a hand-rolled tuple(int(i) ...) skipif (also brittle for pre-release versions). Standardized onto the shared requires_module mark.
  • Updated affected PEP 723 dependency pins to numpy>=2.2.5.

Intentionally left at 2.1:

  • examples/strided_memory_view_constructors.py — only reads DLPack arrays.
  • test_utils.py:129 — guards an unrelated NumPy readonly-input fix (numpy#26501), not the DLPack write issue.

Testing

All edited files compile. The changes are version-guard logic only (no behavior change for supported NumPy); the GPU test suite runs in CI.

Closes #1169

@ArsalanShakil
Fix NVIDIA#1169: correct NumPy skip conditions for DLPack host writes
e092f32 
Writing into a host-accessible array obtained via np.from_dlpack requires
NumPy 2.2.5+, because earlier versions return a read-only array
(numpy GH #28632). Several tests and examples guarded these writes at
NumPy 2.1.0, so on NumPy 2.1.0-2.2.4 they would error instead of skip.

Audit all NumPy version guards in cuda_core and bump the write-into-DLPack
sites to 2.2.5 with a consistent reason matching the existing correct guard
in test_launcher.py. Guards that only read DLPack arrays (e.g.
strided_memory_view_constructors.py) or skip for an unrelated NumPy fix
(test_utils.py, numpy#26501) are left at 2.1.

Also fix examples/memory_ops.py, which used a plain string comparison
(np.__version__ < "2.1.0") that is unreliable for multi-digit versions;
replace it with np.lib.NumpyVersion to match the other examples, and
standardize test_graph_builder.py off a hand-rolled tuple skipif onto
requires_module. Update affected PEP 723 dependency pins to numpy>=2.2.5.

Signed-off-by: Arsalan Shakil <shakil.arsalan@yahoo.com>
@copy-pr-bot

copy-pr-bot Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Jun 18, 2026
@leofang
leofang requested a review from seberg July 9, 2026 03:49
@leofang leofang added test Improvements or additions to tests example Improvements or additions to code examples labels Jul 9, 2026
@leofang leofang added this to the cuda.core next milestone Jul 9, 2026
@leofang leofang added the P1 Medium priority - Should do label Jul 9, 2026
seberg
seberg approved these changes Jul 10, 2026
View reviewed changes

@seberg seberg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, straight forward and looks right. (Comments may be a bit unnecessarily long, but OK.)

@seberg

seberg commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

/ok to test 4a07cb1

@seberg

seberg commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Sorry missed that scrolling through the diff: can you resolve the merge conflict?

@github-actions

This comment has been minimized.

Sign in to view
@ArsalanShakil

ArsalanShakil commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

I will solve the merge conflicts soon, I am away on vacation will be back next end of next week

@ArsalanShakil

ArsalanShakil commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Merge conflict with main is resolved (in cuda_core/tests/graph/test_graph_builder.py): kept the new host-callback survival tests from main and applied the corrected @requires_module(np, "2.2.5", ...) skip condition to test_graph_child_graph. Ready for a re-run whenever you get a chance to /ok to test. Thanks!

@ArsalanShakil
Merge remote-tracking branch 'upstream/main' into fix/1169-numpy-dlpa…
f8ba7d9 
…ck-skip-conditions

# Conflicts:
#	cuda_core/tests/graph/test_graph_builder.py
@ArsalanShakil
ArsalanShakil force-pushed the fix/1169-numpy-dlpack-skip-conditions branch from 9aba315 to f8ba7d9 Compare July 16, 2026 22:05
@seberg

seberg commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

/ok to test f8ba7d9

@seberg
seberg enabled auto-merge (squash) July 17, 2026 13:38
@seberg

seberg commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

/ok to test f8ba7d9

@ArsalanShakil

ArsalanShakil commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

The 3 red checks on f8ba7d9 are unrelated to this PR (which only touches NumPy skip conditions in test_graph_builder.py):

  • Test win-64 / py3.14t, CUDA 13.0.2, a100 (MCDM) — 4 failures, all in graph buffer/slot lifetime tests (test_kernel_args_buffer_lifetime, test_memcpy_buffer_allocations_released_after_graph_destroyed, test_slot_owners_released_after_graph_destroyed, test_pending_call_queue_saturation_preserves_cleanup). Each fails on a 5s timing assertion (_wait_until(lambda: ...) -> AssertionError: condition not satisfied within 5.0s) waiting for weakref/thread finalization after gc.collect() — flaky under free-threaded (no-GIL) Windows. These come from recent graph-cleanup work on main, not this change.
  • Test win-64 / py3.14, CUDA 13.3.0, t4 (TCC) — no pytest failures in the log; a runner/infra-level abort (the runner also logged driver mode MCDM failed and LongPathsEnabled is not set, [BUG]: Windows CI runners need LongPathsEnabled for free-threaded Python #1820).
  • Check job status — just the aggregator, red because of the two jobs above.

This PR's own tests passed. A re-run of the two failed jobs should go green — could you kick one off when convenient? Thanks!

@seberg
seberg merged commit f907e46 into NVIDIA:main Jul 19, 2026
274 of 279 checks passed
@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Doc Preview CI
Preview removed because the pull request was closed or merged.

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

Reviewers

@seberg seberg seberg approved these changes

Assignees

@seberg seberg

@ArsalanShakil ArsalanShakil

Labels

cuda.core Everything related to the cuda.core module example Improvements or additions to code examples P1 Medium priority - Should do test Improvements or additions to tests

Projects

None yet

Milestone

cuda.core 1.2.0

Development

Successfully merging this pull request may close these issues.

Audit all NumPy skip conditions

3 participants

@ArsalanShakil @seberg @leofang