You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/04/01 05:28:26 UTC
[GitHub] [incubator-tvm] kevinthesun opened a new pull request #5200: Fix
intel conv2d auto tune
kevinthesun opened a new pull request #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200
debug_skip_region will cause execution time to be inaccurate on x86. This PR fixes x86 conv2d and depthwise conv2d.
@icemelon9 @anijain2305
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] comaniac commented on issue #5200: Fix intel conv2d
auto tune
Posted by GitBox <gi...@apache.org>.
comaniac commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607397820
Did a brief search and here is a list of TOPI files that has the same use case:
- arm_cpu/conv2d_spatial_pack.py
- arm_cpu/conv2d.py
- arm_cpu/depthwise_conv2d.py
- bifrost/conv2d.py
- cuda/conv2d_int8.py
- cuda/conv2d_winograd.py
- cuda/group_conv2d_nchw.py
- mali/conv2d.py
btw just curious, do you have an experimental result with an isolated case to illustrate the accuracy issue introduced by `debug_skip_region`?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607650219
@merrymercy @FrozenGene Do we keep empty array for now and wait for non_empty array?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request
#5200: Fix intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#discussion_r401955632
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -479,9 +479,10 @@ def run_through_rpc(measure_input, build_result,
if ref_input:
args = [nd.array(x, ctx=ctx) for x in ref_input]
else:
- # create empty arrays on the remote device and copy them once.
+ # create random arrays on the remote device and copy them once.
# This can avoid some memory issues that make the measurement results unreliable.
- args = [nd.empty(x[0], dtype=x[1], ctx=ctx) for x in build_result.arg_info]
+ args = [nd.array(np.random.uniform(0.0, 255.0, size=x[0]).astype(dtype=x[1]), ctx=ctx)
Review comment:
This will introduce a data copy when using RPCRunner, which will bring some network overhead.
One way to solve this is by implementing a `tvm.nd.non_empty` or `tvm.nd.random` in the tvm runtime, then we can do the random fill on the target device without copying over the network.
@FrozenGene has implemented a version in our internal codebase. Maybe @FrozenGene can help on this?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request
#5200: Fix intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#discussion_r401955632
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -479,9 +479,10 @@ def run_through_rpc(measure_input, build_result,
if ref_input:
args = [nd.array(x, ctx=ctx) for x in ref_input]
else:
- # create empty arrays on the remote device and copy them once.
+ # create random arrays on the remote device and copy them once.
# This can avoid some memory issues that make the measurement results unreliable.
- args = [nd.empty(x[0], dtype=x[1], ctx=ctx) for x in build_result.arg_info]
+ args = [nd.array(np.random.uniform(0.0, 255.0, size=x[0]).astype(dtype=x[1]), ctx=ctx)
Review comment:
This will introduce a data copy when using RPCRunner, which will bring too much overhead.
One way to solve this is by implementing a `tvm.nd.non_empty` or `tvm.nd.random` in the tvm runtime, then we can do the random fill on the target device without copying over the network.
@FrozenGene has implemented a version in our internal codebase. Maybe @FrozenGene can help on this?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] anijain2305 commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
anijain2305 commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607350913
@kevinthesun Do you also want to send the PR (or update this one) to change zero tensor to random tensor for AutoTVM?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
merrymercy commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607747123
I am happy with keeping the empty array and merging this first.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607393213
@anijain2305 Added.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
merrymercy commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607530332
Good catch. I can confirm both `tvm.nd.empty` and `debug_skip_region` will cause inaccurate measurement.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun edited a comment on issue #5200: Fix
intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
kevinthesun edited a comment on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607402247
@comaniac One way to verify this is to directly build a tvm func involving debug_skip_region. I verified that on x86 and debug_skip_region did cause inaccurate measurement. However, I didn't dig into why debug_skip_region causes this. For other platforms, @FrozenGene notices this issue also exists. We might want to verify on other platforms and fix them.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request
#5200: Fix intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#discussion_r402013632
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -479,9 +479,10 @@ def run_through_rpc(measure_input, build_result,
if ref_input:
args = [nd.array(x, ctx=ctx) for x in ref_input]
else:
- # create empty arrays on the remote device and copy them once.
+ # create random arrays on the remote device and copy them once.
# This can avoid some memory issues that make the measurement results unreliable.
- args = [nd.empty(x[0], dtype=x[1], ctx=ctx) for x in build_result.arg_info]
+ args = [nd.array(np.random.uniform(0.0, 255.0, size=x[0]).astype(dtype=x[1]), ctx=ctx)
Review comment:
@merrymercy Sure. I will port it to our upstream soon.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy edited a comment on issue #5200: Fix
intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
merrymercy edited a comment on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607530332
Good catch. I can confirm both `tvm.nd.empty` and `debug_skip_region` will cause inaccurate measurement from my experiences.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607388381
@FrozenGene If that's the case, would you mind opening an issue tracking all topi ops we might want to modify?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607402247
@comaniac One way to verify this is to directly build a tvm func involving debug_skip_region. I verified that on x86 and debug_skip_region did cause inaccuracy measurement. However, I didn't dig into why debug_skip_region causes this. For other platforms, @FrozenGene notices this issue also exists. We might want to verify on other platforms and fix them.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-608440722
> I am happy with keeping the empty array and merging this first.
+1
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request
#5200: Fix intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#discussion_r401955632
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -479,9 +479,10 @@ def run_through_rpc(measure_input, build_result,
if ref_input:
args = [nd.array(x, ctx=ctx) for x in ref_input]
else:
- # create empty arrays on the remote device and copy them once.
+ # create random arrays on the remote device and copy them once.
# This can avoid some memory issues that make the measurement results unreliable.
- args = [nd.empty(x[0], dtype=x[1], ctx=ctx) for x in build_result.arg_info]
+ args = [nd.array(np.random.uniform(0.0, 255.0, size=x[0]).astype(dtype=x[1]), ctx=ctx)
Review comment:
This will introduce a data copy when using RPCRunner, which will bring too much overhead.
One way to solve this is implementing a `tvm.nd.non_empty` or `tvm.nd.random` in the tvm runtime, then we can do the random fill on the target device without copying over network
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] anijain2305 edited a comment on issue #5200: Fix
intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
anijain2305 edited a comment on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607350913
@kevinthesun Do you also want to send the PR (or update this one) to change zero tensor to random tensor for AutoTVM for stable measurements?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607062531
I think this issue exist in all auto tvm topi template.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] merrymercy merged pull request #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
merrymercy merged pull request #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-608771757
Is this good to be merged?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request
#5200: Fix intel conv2d auto tune
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#discussion_r402018776
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -479,9 +479,10 @@ def run_through_rpc(measure_input, build_result,
if ref_input:
args = [nd.array(x, ctx=ctx) for x in ref_input]
else:
- # create empty arrays on the remote device and copy them once.
+ # create random arrays on the remote device and copy them once.
# This can avoid some memory issues that make the measurement results unreliable.
- args = [nd.empty(x[0], dtype=x[1], ctx=ctx) for x in build_result.arg_info]
+ args = [nd.array(np.random.uniform(0.0, 255.0, size=x[0]).astype(dtype=x[1]), ctx=ctx)
Review comment:
I open https://github.com/apache/incubator-tvm/issues/5216 to track this
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #5200: Fix intel
conv2d auto tune
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on issue #5200: Fix intel conv2d auto tune
URL: https://github.com/apache/incubator-tvm/pull/5200#issuecomment-607581696
Open https://github.com/apache/incubator-tvm/issues/5215 to track this issue.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services