Skip to content

Conversation

@cpcloud
Copy link
Contributor

@cpcloud cpcloud commented Oct 24, 2025

Closes #617.

@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Oct 24, 2025

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@cpcloud
Copy link
Contributor Author

cpcloud commented Oct 24, 2025

/ok to test

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR fixes a parameter validation bug in the cuStreamBeginCaptureToGraph binding. The function accepts an optional dependencyData parameter that can be None according to the CUDA API specification, but the implementation was incorrectly enforcing a length check against numDependencies for this optional parameter. The removed validation line (37008) was causing RuntimeErrors when users legitimately passed None for dependencyData. The fix removes this incorrect validation while preserving the necessary length check for the required dependencies parameter. This change aligns the Python bindings in cuda_bindings with the CUDA driver API specification and resolves issue #617.

Important Files Changed

Filename Score Overview
cuda_bindings/cuda/bindings/driver.pyx.in 5/5 Removed incorrect validation for optional dependencyData parameter in cuStreamBeginCaptureToGraph function

Confidence score: 5/5

  • This PR is safe to merge with minimal risk
  • Score reflects a straightforward bug fix that removes incorrect validation logic for an optional parameter without introducing new complexity or side effects; the change correctly aligns the bindings with CUDA API semantics
  • No files require special attention

Sequence Diagram

sequenceDiagram
    participant User
    participant "Python Binding" as Binding
    participant "CUDA Driver API" as CUDA
    participant "GPU Stream" as Stream
    participant "CUDA Graph" as Graph

    User->>Binding: "cuStreamBeginCaptureToGraph(hStream, hGraph, dependencies, dependencyData, numDependencies, mode)"
    Note over User,Binding: dependencyData can now be None/optional
    Binding->>Binding: "Validate parameters"
    alt dependencyData is None
        Binding->>CUDA: "cuStreamBeginCaptureToGraph(hStream, hGraph, dependencies, NULL, numDependencies, mode)"
    else dependencyData is provided
        Binding->>CUDA: "cuStreamBeginCaptureToGraph(hStream, hGraph, dependencies, dependencyData, numDependencies, mode)"
    end
    CUDA->>Stream: "Begin capture mode"
    Stream->>Graph: "Capture operations to graph"
    CUDA-->>Binding: "Return CUDA result"
    Binding-->>User: "Return result or raise exception"
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link

@cpcloud cpcloud requested review from leofang, mdboom and rwgk October 24, 2025 16:47
Copy link
Contributor

@mdboom mdboom left a comment

Choose a reason for hiding this comment

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

LGTM

@cpcloud cpcloud enabled auto-merge (squash) October 24, 2025 17:12
@cpcloud cpcloud merged commit 9f1269b into NVIDIA:main Oct 24, 2025
64 checks passed
@rparolin rparolin added this to the cuda.core beta 8 milestone Oct 24, 2025
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.

[BUG]: cuStreamBeginCaptureToGraph dependencyData should be optional

3 participants