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

Add available_device_memory to fetch free amount of memory on a GPU #1567

Merged
merged 11 commits into from
Jun 4, 2024

Conversation

galipremsagar
Copy link
Contributor

@galipremsagar galipremsagar commented May 21, 2024

Description

This PR adds get_free_device_memory that returns free GPU memory necessary for rapidsai/cudf#15628

Checklist

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

@galipremsagar galipremsagar self-assigned this May 21, 2024
@galipremsagar galipremsagar requested a review from a team as a code owner May 21, 2024 14:36
@galipremsagar galipremsagar added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels May 21, 2024
@github-actions github-actions bot added the Python Related to RMM Python API label May 21, 2024
@galipremsagar galipremsagar changed the base branch from branch-24.06 to branch-24.08 May 21, 2024 23:32
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.

Just one question.

python/rmm/rmm/_lib/memory_resource.pyx Outdated Show resolved Hide resolved
@@ -1188,3 +1188,10 @@ def get_log_filenames():
else None
for i, each_mr in _per_device_mrs.items()
}


def get_free_device_memory(percent):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: let's make this parameter self-documenting with:

def get_free_device_memory(*, percent: int):

WDYT?

python/rmm/rmm/_lib/memory_resource.pyx Outdated Show resolved Hide resolved
python/rmm/rmm/_lib/memory_resource.pyx Outdated Show resolved Hide resolved
@galipremsagar galipremsagar requested review from wence-, bdice and harrism May 28, 2024 19:22
@galipremsagar galipremsagar requested a review from bdice May 28, 2024 23:00
python/rmm/rmm/_lib/memory_resource.pyx Outdated Show resolved Hide resolved
python/rmm/rmm/tests/test_rmm.py Outdated Show resolved Hide resolved
@galipremsagar galipremsagar requested a review from bdice May 29, 2024 14:33
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. Thanks.

@harrism harrism changed the title Add get_free_device_memory to fetch free amount of memory on a GPU Add available_device_memory to fetch free amount of memory on a GPU May 30, 2024
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

One more round of comments -- but I think this can be merged after you fix the import statement, so I'll approve.

python/rmm/rmm/tests/test_rmm.py Outdated Show resolved Hide resolved
python/rmm/rmm/tests/test_rmm.py Outdated Show resolved Hide resolved

def available_device_memory():
"""
Returns a tuple of free and total device memory memory.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wish this was in a Returns block that follows numpy docstring style but it doesn't seem like RMM uses full docstrings...

Copy link
Member

Choose a reason for hiding this comment

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

RMM Python is immature in a lot of ways. No time like the present...

@galipremsagar
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 8597c22 into rapidsai:branch-24.08 Jun 4, 2024
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Related to RMM Python API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants