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.

Sign up for GitHub

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

Jump to bottom

Add avg and std calculation for time and throughput #828

Merged
merged 9 commits into from
Jan 12, 2022
Merged

Add avg and std calculation for time and throughput #828

merged 9 commits into from
Jan 12, 2022

Conversation

quasiben
Copy link
Member

PR adds AVG/STD calc for time and throughput

===============================
Wall-clock     | Throughput
-------------------------------
294.73 ms      | 82.83 MiB/s
220.04 ms      | 110.95 MiB/s
247.50 ms      | 98.64 MiB/s
214.36 ms      | 113.89 MiB/s
224.59 ms      | 108.71 MiB/s
231.22 ms      | 105.59 MiB/s
228.30 ms      | 106.94 MiB/s
296.44 ms      | 82.36 MiB/s
226.30 ms      | 107.89 MiB/s
221.86 ms      | 110.04 MiB/s
===============================
Throughput | 102.78 MiB +/- 10.78 MiB
Wall-Clock | 240.53 ms +/- 28.74 ms

@quasiben quasiben requested a review from a team as a code owner January 12, 2022 15:57
@github-actions github-actions bot added the python python code needed label Jan 12, 2022
@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 12, 2022
pentschev
pentschev reviewed Jan 12, 2022
View reviewed changes
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.

Perhaps these changes should do it.

dask_cuda/benchmarks/local_cudf_merge.py Outdated Show resolved Hide resolved
dask_cuda/benchmarks/local_cudf_merge.py Outdated Show resolved Hide resolved
dask_cuda/benchmarks/local_cudf_merge.py Outdated Show resolved Hide resolved
dask_cuda/benchmarks/local_cudf_merge.py Outdated Show resolved Hide resolved
dask_cuda/benchmarks/local_cudf_merge.py Outdated Show resolved Hide resolved
@quasiben
Copy link
Member Author

New changes look like the following:

===============================
Wall-clock     | Throughput
-------------------------------
304.65 ms      | 80.14 MiB/s
226.29 ms      | 107.89 MiB/s
227.56 ms      | 107.28 MiB/s
221.32 ms      | 110.31 MiB/s
229.52 ms      | 106.37 MiB/s
234.61 ms      | 104.06 MiB/s
228.25 ms      | 106.96 MiB/s
225.67 ms      | 108.19 MiB/s
214.79 ms      | 113.67 MiB/s
221.88 ms      | 110.03 MiB/s
===============================
Throughput | 105.49 MiB +/- 8.80 MiB
Wall-Clock | 233.45 ms +/- 24.27 ms
===============================

@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2022 •
edited
Loading

Codecov Report

Merging #828 (b4b869b) into branch-22.02 (172e449) will decrease coverage by 0.14%.
The diff coverage is n/a.

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

@@               Coverage Diff                @@
##           branch-22.02     #828      +/-   ##
================================================
- Coverage         87.07%   86.92%   -0.15%     
================================================
  Files                16       16              
  Lines              2065     2065              
================================================
- Hits               1798     1795       -3     
- Misses              267      270       +3
Impacted Files Coverage Δ
dask_cuda/utils.py 82.02% <0.00%> (-1.13%) ⬇️

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 172e449...f95be6f. Read the comment docs.

@pentschev
Copy link
Member

Did you paste the right one in #828 (comment) @quasiben ? Seems exactly the same as before.

@quasiben
Copy link
Member Author

@pentschev I think i've got all the issues resolved. When you have time can you take another look ?

pentschev
pentschev approved these changes Jan 12, 2022
View reviewed changes
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 @quasiben !

@pentschev
Copy link
Member

@gpucibot merge

1 similar comment
@pentschev
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 823baec into rapidsai:branch-22.02 Jan 12, 2022
@quasiben quasiben deleted the add-avg-std branch March 21, 2022 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@galipremsagar galipremsagar galipremsagar approved these changes

@pentschev pentschev pentschev approved these changes

Assignees
No one assigned
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python python code needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@quasiben @codecov-commenter @pentschev @galipremsagar