-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[MXNET-91] Added unittest for benchmarking metric performance #9705
Conversation
@szha Please review. |
Why not include small batch size like 64? 100k is huge. |
The intention is to observe a measurable elapsed time (hence large data size) and amplify the difference between CPU and GPU processing (hence processing all the data in one batch). A valid alternative is to use small batch size and iterate over multiple batches. |
11a59cc
to
5d19aef
Compare
@safrooze This is the wrong reasoning - the fact that GPU is faster than CPU when processing million elements does not mean you should use GPU when adding 2 numbers together. You should only test on batch sizes that are realistic (and do multiple runs to have measurable time difference). |
Please make sure to use a fixed seed in order to provide reproducibility in between different runs. |
OK I think I addressed all the feedback:
The modified code output looks like this:
|
Wouldn't nightly test be a better place for performance tests like this? This unit test doesn't verify the correctness at all. |
@eric-haibin-lin You're correct that nightly would be a more suitable place. One concern with nightly was that community wouldn't be able to see the results of the benchmark. |
It is planned to move the nightly tests to the public CI very soon.
Am 09.02.2018 7:28 nachm. schrieb "Sina Afrooze" <notifications@github.com>:
… @eric-haibin-lin <https://github.com/eric-haibin-lin> You're correct that
nightly would be a more suitable place. One concern with nightly was that
community wouldn't be able to see the results of the benchmark.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#9705 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARxB6xpawGZ70PgaUdyVdYgBCkKfiv_gks5tTI5cgaJpZM4R6dXt>
.
|
When will nightly tests be moved to public CI? |
It's planned for end of Q1
Am 20.02.2018 5:47 vorm. schrieb "Sheng Zha" <notifications@github.com>:
… When will nightly tests be moved to public CI?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#9705 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARxB64_1KvbTT_mTlGJN8Ag9V6x2bLG6ks5tWk5vgaJpZM4R6dXt>
.
|
I'll move this to nightly tests then. |
Hi, the community has passed to vote about associating the code changes with JIRA (https://lists.apache.org/thread.html/ab22cf0e35f1bce2c3bf3bec2bc5b85a9583a3fe7fd56ba1bbade55f@%3Cdev.mxnet.apache.org%3E) We have updated the guidelines for contributors in https://cwiki.apache.org/confluence/display/MXNET/Development+Process, please ensure that you have created a JIRA at https://issues.apache.org/jira/projects/MXNET/issues/ to describe your work in this pull request and include the JIRA title in your PR as [MXNET-xxxx] your title where MXNET-xxxx is the JIRA id Thanks! |
Checking in on the public nightly build results, is it still on track? |
I don't think so - at least not from my side. We have been resource constrained and managing the Nightly CI does not fit into my schedule, especially since all jobs have to be refactored. I will ask Bhavins Team to do it and I will do the reviews, but I personally am not able to refactor that part as well. On the other hand, we've got additional headcount approved for CI, but it will take some time until everybody is ramped up. We will have to see how and when we can continue. |
In that case, let's put the test in unittest for now. @safrooze could you resolve conflict? |
One last request: would you put the performance tests in a separate test file, such as test_metric_perf.py, so that it's easier to move to nightly later? |
- Output of the benchmark is sent to stderr - random is seeded - nd.wait_all() used before starting timing and before ending timing - Added batch-size values of 16, 64, 256, and 1024 - Datasize varies by number of output channels to keep total runtime down to a few minutes
33e161d
to
4d369e5
Compare
- Output of the benchmark is sent to stderr - random is seeded - nd.wait_all() used before starting timing and before ending timing - Added batch-size values of 16, 64, 256, and 1024 - Datasize varies by number of output channels to keep total runtime down to a few minutes
- Output of the benchmark is sent to stderr - random is seeded - nd.wait_all() used before starting timing and before ending timing - Added batch-size values of 16, 64, 256, and 1024 - Datasize varies by number of output channels to keep total runtime down to a few minutes
- Output of the benchmark is sent to stderr - random is seeded - nd.wait_all() used before starting timing and before ending timing - Added batch-size values of 16, 64, 256, and 1024 - Datasize varies by number of output channels to keep total runtime down to a few minutes
Why does this test only print numbers but doesn't actually enforce anything? |
Output of the benchmark is sent to stderr
Description
Benchmark loops through two batch-sizes (100,000 and 1,000,000) and two output dimensions (100 and 500) and generates random data on CPU and GPU and calls metric.update() on a list of metrics with the generated date.
Checklist
Essentials
make lint
)Changes
Comments