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 device memory limit argument to benchmarks #683

Merged
merged 3 commits into from
Jul 23, 2021

Conversation

charlesbluca
Copy link
Member

Adds a --device-memory-limit argument to the benchmark utils, to set a device memory limit on the instantiated LocalCUDACluster. This can be helpful when we want to force spilling to host (for context, I am doing this while benchmarking pack/unpack serialization).

@charlesbluca charlesbluca added python python code needed 3 - Ready for Review Ready for review by team feature request New feature or request non-breaking Non-breaking change labels Jul 23, 2021
@charlesbluca charlesbluca requested a review from a team as a code owner July 23, 2021 14:36
@pentschev
Copy link
Member

rerun tests

@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2021

Codecov Report

Merging #683 (47e055b) into branch-21.10 (c04856e) will increase coverage by 2.47%.
The diff coverage is n/a.

❗ Current head 47e055b differs from pull request most recent head ea19586. Consider uploading reports for the commit ea19586 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.10     #683      +/-   ##
================================================
+ Coverage         87.83%   90.30%   +2.47%     
================================================
  Files                15       15              
  Lines              1652     1651       -1     
================================================
+ Hits               1451     1491      +40     
+ Misses              201      160      -41     
Impacted Files Coverage Δ
dask_cuda/explicit_comms/dataframe/shuffle.py 98.69% <0.00%> (+0.65%) ⬆️
dask_cuda/proxy_object.py 90.73% <0.00%> (+1.08%) ⬆️
dask_cuda/utils.py 88.94% <0.00%> (+1.32%) ⬆️
dask_cuda/cli/dask_cuda_worker.py 97.14% <0.00%> (+1.42%) ⬆️
dask_cuda/device_host_file.py 71.77% <0.00%> (+1.61%) ⬆️
dask_cuda/initialize.py 92.59% <0.00%> (+3.70%) ⬆️
dask_cuda/proxify_device_objects.py 95.72% <0.00%> (+6.83%) ⬆️
dask_cuda/get_device_memory_objects.py 87.50% <0.00%> (+19.44%) ⬆️
dask_cuda/is_device_object.py 88.88% <0.00%> (+25.92%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c04856e...ea19586. Read the comment docs.

@@ -202,6 +212,8 @@ def get_cluster_options(args):
worker_options["enable_infiniband"] = ""
if args.enable_rdmacm:
worker_options["enable_rdmacm"] = ""
if args.device_memory_limit:
worker_options["device_memory_limit"] = ""
Copy link
Member

Choose a reason for hiding this comment

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

Now it's missing the value to be assigned, which I believe should be args.device_memory_limit.

Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @charlesbluca !

@pentschev
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit e60616d into rapidsai:branch-21.10 Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team feature request New feature or request non-breaking Non-breaking change python python code needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants