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/09/03 08:04:11 UTC
[GitHub] [incubator-tvm] FrozenGene opened a new pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
FrozenGene opened a new pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391
Follow up #5913 and #5914 , enable AutoTVM / Ansor support random fill and CPU cache flush to get preciser measurement.
@merrymercy @jcf94
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483151525
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -657,9 +692,11 @@ def timed_func():
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Since currently `USE_RANDOM` is still a config, I would say we should have such a guard to make the code base more comprehensive. Imagining someone never updates `build/config.cmake` and his `USE_RANDOM` is OFF, then he may encounter crashing without a proper message. We can for example simply throw out a warning saying "USE_RANDOM is OFF and it might lead to inaccurate measurements" to work around this issue IMO.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483899674
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -511,10 +511,12 @@ 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.
- # This can avoid some memory issues that make the measurement results unreliable.
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Please make sure USE_RANDOM is ON in the config.cmake"
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
If the check is on the device. L514 is useless.
In addition, we should use try-catch to capture the error from remote devices and explicitly use "remote" in the error message.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483101547
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -657,9 +692,11 @@ def timed_func():
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Seems to me that this symbol will be missing if `USE_RANDOM` is 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:
users@infra.apache.org
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483899674
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -511,10 +511,12 @@ 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.
- # This can avoid some memory issues that make the measurement results unreliable.
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Please make sure USE_RANDOM is ON in the config.cmake"
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
The check should be done on the remote device, so L514 is useless.
In addition, we should use try-catch to capture the error from remote devices and mention "remote" in the error message. e.g.,
```Please make sure USE_RANDOM is ON in the config.cmake on the remote devices```
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483201157
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -657,9 +692,11 @@ def timed_func():
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Works for me. Thanks.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483593913
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -566,9 +592,13 @@ def timed_func(inp, build_res):
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Do you enable USE_RANDOM in the config.cmake?"
+ random_fill = tvm.get_global_func("tvm.contrib.random.random_fill")
Review comment:
```suggestion
random_fill = tvm.get_global_func("tvm.contrib.random.random_fill", True)
assert random_fill, "Please set USE_RANDOM to On the config.cmake."
```
We should call tvm.get_global_func only once.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483416557
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -657,9 +692,11 @@ def timed_func():
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
done.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483200130
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -657,9 +692,11 @@ def timed_func():
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Yes, we could add one message. As `random fill` is the base if we want to get precise measure result, I would like to add one assert to make sure we have this function. Prompt users you should turn on `USE_RANDOM` and not just warning to make it pass. Does it make sense to you?
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483899527
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -697,7 +696,7 @@ def timed_func():
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
- "Do you enable USE_RANDOM in the config.cmake?"
+ "Please make sure USE_RANDOM is ON in the config.cmake"
random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Then we should remove the assert `assert tvm.get_global_func("tvm.contrib.random.random_fill", True)` and use some try catch to capture the error from devices.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r484056896
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -511,10 +511,12 @@ 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.
- # This can avoid some memory issues that make the measurement results unreliable.
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Please make sure USE_RANDOM is ON in the config.cmake"
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
done.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483631435
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -566,9 +592,13 @@ def timed_func(inp, build_res):
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Do you enable USE_RANDOM in the config.cmake?"
+ random_fill = tvm.get_global_func("tvm.contrib.random.random_fill")
Review comment:
done.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483593913
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -566,9 +592,13 @@ def timed_func(inp, build_res):
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Do you enable USE_RANDOM in the config.cmake?"
+ random_fill = tvm.get_global_func("tvm.contrib.random.random_fill")
Review comment:
```suggestion
random_fill = tvm.get_global_func("tvm.contrib.random.random_fill", True)
assert random_fill, "Please set USE_RANDOM to ON the config.cmake."
```
We should call tvm.get_global_func only once.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483743676
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -697,7 +696,7 @@ def timed_func():
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
- "Do you enable USE_RANDOM in the config.cmake?"
+ "Please make sure USE_RANDOM is ON in the config.cmake"
random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Here `remote.get_function` can not be used for checking like `tvm.get_global_func`
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483200130
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -657,9 +692,11 @@ def timed_func():
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Yes, we could add one message. As `random fill` is the base if we want to get precise measure result, I would like to add one assert to make sure we have this function. Prompt users you should turn on `USE_RANDOM` and not just throw one warning to make it pass. Does it make sense to you?
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy merged pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy merged pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483737527
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -697,7 +696,7 @@ def timed_func():
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
- "Do you enable USE_RANDOM in the config.cmake?"
+ "Please make sure USE_RANDOM is ON in the config.cmake"
random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Here still calls twice.
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -512,7 +512,7 @@ def run_through_rpc(measure_input, build_result,
args = [nd.array(x, ctx=ctx) for x in ref_input]
else:
assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
- "Do you enable USE_RANDOM in the config.cmake?"
+ "Please make sure USE_RANDOM is ON in the config.cmake"
random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
ditto.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483592006
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -566,9 +592,13 @@ def timed_func(inp, build_res):
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Do you enable USE_RANDOM in the config.cmake?"
+ random_fill = tvm.get_global_func("tvm.contrib.random.random_fill")
Review comment:
We should call `tvm.get_global_func` only once
```
random_fill = tvm.get_global_func("tvm.contrib.random.random_fill", True)
assert random_fill is not None, "Please set USE_RANDOM to ON in the config.cmake"
```
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483592006
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -566,9 +592,13 @@ def timed_func(inp, build_res):
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Do you enable USE_RANDOM in the config.cmake?"
+ random_fill = tvm.get_global_func("tvm.contrib.random.random_fill")
Review comment:
We should call `tvm.get_global_func` only once
```
random_fill = tvm.get_global_func("tvm.contrib.random.random_fill", True)
assert random_fill is not None, "Do you enable USE_RANDOM in the config.cmake?"
```
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483899674
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -511,10 +511,12 @@ 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.
- # This can avoid some memory issues that make the measurement results unreliable.
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Please make sure USE_RANDOM is ON in the config.cmake"
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
The check should be done on the remote device, so L514 is useless.
In addition, we should use try-catch to capture the error from remote devices and explicitly use error message like
```Please make sure USE_RANDOM is ON in the config.cmake on the remote devices```
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483906265
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -511,10 +511,12 @@ 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.
- # This can avoid some memory issues that make the measurement results unreliable.
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Please make sure USE_RANDOM is ON in the config.cmake"
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
Good point. Thanks for pointing this. I will update implementation for rpc.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] FrozenGene commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
FrozenGene commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483128177
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -657,9 +692,11 @@ def timed_func():
if error_no == 0:
try:
- # TODO(FrozenGene): Update to ndarray.non-empty.
args = [ndarray.empty(get_const_tuple(x.shape), x.dtype, ctx) for x in
build_res.args]
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
I think about it for a while when I implemented it. One possible way to support both ways is:
```python
if not tvm.get_global_func("tvm.contrib.random.random_fill", True):
args = [np.empty()...]
else
args = [np.empty()...]
random_fill(value)
```
However, `empty` can not make us get precise measure result. And as `USE_RANDOM` is set to be `ON` by default and we don't have any special reasons to turn it off until now (like we won't turn off `USE_GRAPHRUNTIME`), so I prefer only maintain current code implementation. Of course, we could have one debate here. But IMO, I like we expose the error in compilation (for example, miss symbol) rather than make it pass but get the result we don't like (for example we go to the way of `empty` but get not enough precise measure result).
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483899674
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -511,10 +511,12 @@ 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.
- # This can avoid some memory issues that make the measurement results unreliable.
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Please make sure USE_RANDOM is ON in the config.cmake"
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
The check should be done on the remote device, so L514 is useless.
In addition, we should use try-catch to capture the error from remote devices and mention "remote" in the error message. e.g.,
```Please make sure USE_RANDOM is ON in the config.cmake on the remote devices```
This is important because `config.cmake` on remote devices won't be updated by `git pull` and rebuild.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6391: [AutoTVM][Ansor] Enable random fill and CPU cache flush for AutoTVM and Ansor
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6391:
URL: https://github.com/apache/incubator-tvm/pull/6391#discussion_r483899674
##########
File path: python/tvm/autotvm/measure/measure_methods.py
##########
@@ -511,10 +511,12 @@ 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.
- # This can avoid some memory issues that make the measurement results unreliable.
+ assert tvm.get_global_func("tvm.contrib.random.random_fill", True), \
+ "Please make sure USE_RANDOM is ON in the config.cmake"
+ random_fill = remote.get_function("tvm.contrib.random.random_fill")
Review comment:
If the check is on the device. L512 is useless.
In addition, we should use try-catch to capture the error from remote devices and explicitly use "remote" in the error message.
----------------------------------------------------------------
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