Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept stream argument in DeviceMemoryResource allocate/deallocate #1494

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

wence-
Copy link
Contributor

@wence- wence- commented Mar 5, 2024

Description

In #980, the DeviceMemoryResource class in Python gained allocation and deallocation routines. This was to facilitate writing Python allocate/deallocate callbacks for the CallbackMemoryResource.

These routines should, to match the C++ API, accept a stream parameter such that one can use them for stream-ordered allocation. Although we recommend that users allocate on the Python side using the DeviceBuffer interface, exposing these routines implicitly makes them public.

To fix this, add an optional stream argument defaulting to the default stream.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

In rapidsai#980, the DeviceMemoryResource class in Python gained allocation
and deallocation routines. This was to facility writing Python
allocate/deallocate callbacks for the CallbackMemoryResource.

These routines should, to match the C++ API, accept a stream parameter
such that one can use them for stream-ordered allocation. Although we
recommend that users allocate on the Python side using the
DeviceBuffer interface, exposing these routines implicitly makes them
public.

To fix this, add an optional stream argument defaulting to the default
stream.

- Closes rapidsai#1493
@wence- wence- added bug Something isn't working breaking Breaking change labels Mar 5, 2024
@wence- wence- requested a review from a team as a code owner March 5, 2024 10:45
@github-actions github-actions bot added the Python Related to RMM Python API label Mar 5, 2024
@wence-
Copy link
Contributor Author

wence- commented Mar 5, 2024

This is a breaking change since any user of the CallbackMemoryResource must adapt their callback signatures. That said, I can't find any...

Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Looks good, just one comment. Thank you.

python/rmm/_lib/memory_resource.pyx Show resolved Hide resolved
@wence-
Copy link
Contributor Author

wence- commented Mar 7, 2024

/merge

@rapids-bot rapids-bot bot merged commit e173101 into rapidsai:branch-24.04 Mar 7, 2024
53 checks passed
@wence- wence- deleted the wence/fea/1493 branch March 7, 2024 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change bug Something isn't working Python Related to RMM Python API
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUG] Python memory_resource allocate/deallocate do not accept a stream argument
4 participants