[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-589927989 Hi @TaoLv and @PatricZhao I reverted my last commit of "[Do not use bit-mask when MKL dropout is used](https://github.com/apache/incubator-mxnet/pull/16735/commits/746a8f09c0ba72858f2dc08644f52ac20a02fbe7)." It makes the code too bristle and also involves very complicate logic to check memory allocation at runtime. Here are the main reasons: (1) MKL dropout support is currently not complete. It does not work if the input data type is smaller than int32 and it does not support broadcast option (when the option axes is specified). This limitation enforces a check at runtime which is not possible in the InferShape function e.g. In this function, I will need to check if the dtype is greater than int32 in order to use a different shape for MKL Dropout. https://github.com/apache/incubator-mxnet/pull/16735/files#diff-74c4dc433970c5df31a5e2c4b57c8d71R127 (2) Having different Dropout engine at runtime (based on data type and ) may cause inconsistency in the mixed precision case. Introducing another difference in mask memory allocation complicates this even further. I think we should focus on enhancing MKL Dropout so that it (1) supports all the different cases as non MKL dropout (2) supports bit-mask. Please let me know what you think. Thanks! Lin This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-589927989 Hi @TaoLv and @PatricZhao I reverted my last commit [https://github.com/apache/incubator-mxnet/pull/16735/commits/746a8f09c0ba72858f2dc08644f52ac20a02fbe7](url) It makes the code too bristle and also involves very complicate logic to check memory allocation at runtime. Here are the main reasons: (1) MKL dropout support is currently not complete. It does not work if the input data type is smaller than int32 and it does not support broadcast option (when the option axes is specified). This limitation enforces a check at runtime which is not possible in the InferShape function e.g. In this function, I will need to check if the dtype is greater than int32 in order to use a different shape for MKL Dropout. https://github.com/apache/incubator-mxnet/pull/16735/files#diff-74c4dc433970c5df31a5e2c4b57c8d71R127 (2) Having different Dropout engine at runtime (based on data type and ) may cause inconsistency in the mixed precision case. Introducing another difference in mask memory allocation complicates this even further. I think we should focus on enhancing MKL Dropout so that it (1) supports all the different cases as non MKL dropout (2) supports bit-mask. Please let me know what you think. Thanks! Lin This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-589927989 Hi @TaoLv and @PatricZhao I reverted my last commit " It makes the code too bristle and also involves very complicate logic to check memory allocation at runtime. Here are the main reasons: (1) MKL dropout support is currently not complete. It does not work if the input data type is smaller than int32 and it does not support broadcast option (when the option axes is specified). This limitation enforces a check at runtime which is not possible in the InferShape function e.g. In this function, I will need to check if the dtype is greater than int32 in order to use a different shape for MKL Dropout. https://github.com/apache/incubator-mxnet/pull/16735/files#diff-74c4dc433970c5df31a5e2c4b57c8d71R127 (2) Having different Dropout engine at runtime (based on data type and ) may cause inconsistency in the mixed precision case. Introducing another difference in mask memory allocation complicates this even further. I think we should focus on enhancing MKL Dropout so that it (1) supports all the different cases as non MKL dropout (2) supports bit-mask. Please let me know what you think. Thanks! Lin This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-586650517 > > Does the slow down come from more computation in the new algorithm or the sub-optimal implementation? > > The new implementation increases both memory load and additional bit-wise operations. So performance slow down is expected. The memory load is actually reduced even in the case of MKL, right? Please refer to the tests results in the PR description. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-586519348 > Does the slow down come from more computation in the new algorithm or the sub-optimal implementation? @PatricZhao The slowdown comes from extra computation in the new algorithm when Dropout uses MKL implementation. MKL already computed the mask but stored each mask as integer. The new algorithm simply repackage this int32 based mask into bit-based mask and therefore introduced extra runtime. In the ideal case, it would be to enhance MKL dropout to store mask using bits. But it requires modification of the VSL APIs. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout
operator
URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-586062120
@roywei Using the test script in
https://github.com/apache/incubator-mxnet/pull/13896
Build | runtime (before) | runtime (after)
---|---|---
CPU w/ MKL | 262 ms ± 1.2 ms | 337 ms ± 12.5 ms
CPU w/o MKL |359 ms ± 241 µs | 426 ms ± 222 µs
GPU w/ cuDNN | 25.9 ms ± 202 µs | 25.8 ms ± 183 µs
GPU w/o cuDNNN |1.34 s ± 5.83 ms | 1.35 s ± 13.1 ms
Using python timer to measure CPU performance with MKL:
This PR:
```
[{'Dropout': [{'avg_time_Dropout': 1.1714265774935484, 'p50_time_Dropout':
1.1715246364474297, 'p90_time_Dropout': 1.190436165779829, 'p99_time_Dropout':
1.2154309218749404, 'inputs': {'data': (1024, 1024)}}]}]
```
Master:
```
[{'Dropout': [{'avg_time_Dropout': 0.6394564639776945, 'p50_time_Dropout':
0.6996351294219494, 'p90_time_Dropout': 1.045508868992329, 'p99_time_Dropout':
1.59036863129586, 'inputs': {'data': (1024, 1024)}}]}]
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout
operator
URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-586062120
@roywei Using the test script in
https://github.com/apache/incubator-mxnet/pull/13896
Build | runtime (before) | runtime (after)
---|---|---
CPU w/ MKL | 262 ms ± 1.2 ms | 337 ms ± 12.5 ms
Using python timer to measure CPU performance with MKL:
This PR:
```
[{'Dropout': [{'avg_time_Dropout': 1.1714265774935484, 'p50_time_Dropout':
1.1715246364474297, 'p90_time_Dropout': 1.190436165779829, 'p99_time_Dropout':
1.2154309218749404, 'inputs': {'data': (1024, 1024)}}]}]
```
Master:
```
[{'Dropout': [{'avg_time_Dropout': 0.6394564639776945, 'p50_time_Dropout':
0.6996351294219494, 'p90_time_Dropout': 1.045508868992329, 'p99_time_Dropout':
1.59036863129586, 'inputs': {'data': (1024, 1024)}}]}]
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout
operator
URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-584766500
@TaoLv Thanks for your review and suggestion.
Using blocking does help. This is the new runtime using python timer:
```
[{'Dropout': [{'avg_time_Dropout': 2.9556092573329806, 'p50_time_Dropout':
2.9536145739257336, 'p90_time_Dropout': 2.9735330026596785, 'p99_time_Dropout':
3.0410749791190033, 'inputs': {'data': (1024, 1024)}}]}]
```
I also updated the runtime using mxnet profiler w/ my performance script in
the PR description section. Please refer above.
I am not in favor of adding an option. It exposes internal implementation
(e.g. using bit mask or not) to users and is also counterintuitive (why not use
bit-mask). If sacrificing performance (to some extent) can help improve
usability, I think we need to consider the trade off.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] apeforest edited a comment on issue #16735: Use single-bit for mask in dropout operator
apeforest edited a comment on issue #16735: Use single-bit for mask in dropout
operator
URL: https://github.com/apache/incubator-mxnet/pull/16735#issuecomment-572830100
@TaoLv Thanks for your review. I ran operator profiling using
benchmark.opperf.utils.benchmark_utils.run_performance_test. The result shows
speed up in forward but some degradation in backward pass.
w/ this change:
```
[{'Dropout': [{'avg_time_forward_Dropout': 1.3266,
'max_storage_mem_alloc_cpu/0': 4259.8398, 'avg_time_backward_Dropout': 0.2682,
'inputs': {'data': (1024, 1024), 'p': 0.5}}]}]
```
w/o this change:
```
[{'Dropout': [{'avg_time_forward_Dropout': 1.7864,
'max_storage_mem_alloc_cpu/0': 6291.4561, 'avg_time_backward_Dropout': 0.1836,
'inputs': {'data': (1024, 1024), 'p': 0.5}}]}]
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
