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 2022/07/13 16:19:01 UTC
[GitHub] [tvm] quic-sanirudh opened a new pull request, #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
quic-sanirudh opened a new pull request, #12082:
URL: https://github.com/apache/tvm/pull/12082
This is the first of a few PRs I'll raise to update the hexagon tests pylint compliant.
This patch fixes the below 7 files
* [X] tests/python/contrib/test_hexagon/benchmark_elemwise_add.py
* [X] tests/python/contrib/test_hexagon/benchmark_util.py
* [X] tests/python/contrib/test_hexagon/conftest.py
* [X] tests/python/contrib/test_hexagon/conv2d/test_conv2d_blocked.py
* [X] tests/python/contrib/test_hexagon/conv2d/test_conv2d_conv2d.py
* [X] tests/python/contrib/test_hexagon/infrastructure.py
* [X] tests/python/contrib/test_hexagon/test_2d_physical_buffers.py
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #12082:
URL: https://github.com/apache/tvm/pull/12082#issuecomment-1189189976
Landing this as it's a great step towards linted tests - thanks @quic-sanirudh!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] quic-sanirudh commented on pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
quic-sanirudh commented on PR #12082:
URL: https://github.com/apache/tvm/pull/12082#issuecomment-1184400210
@mehrdadh @cconvey @adstraw Could you take a look at this PR to make the tests pylint compliant when you get a chance. based on git history, I saw that the 3 of you are the top authors to these files, so I thought I'll request your help in reviewing 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on a diff in pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
Mousius commented on code in PR #12082:
URL: https://github.com/apache/tvm/pull/12082#discussion_r924643330
##########
tests/python/contrib/test_hexagon/test_2d_physical_buffers.py:
##########
@@ -84,6 +83,12 @@ def target_host(target):
return tvm.target.Target(target, host=host)
+# Disabling redefined-outer-name for the whole file as there isn't any easy
+# solution yet to refactor tvm.testing.fixture fixtures that avoid redefining
+# outer variable names
+# pylint: disable=redefined-outer-name
Review Comment:
Argh, sorry, I just looked again and see this is a bit more rework than I originally thought.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius merged pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
Mousius merged PR #12082:
URL: https://github.com/apache/tvm/pull/12082
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] quic-sanirudh commented on a diff in pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
quic-sanirudh commented on code in PR #12082:
URL: https://github.com/apache/tvm/pull/12082#discussion_r923628416
##########
tests/python/contrib/test_hexagon/test_2d_physical_buffers.py:
##########
@@ -84,6 +83,12 @@ def target_host(target):
return tvm.target.Target(target, host=host)
+# Disabling redefined-outer-name for the whole file as there isn't any easy
+# solution yet to refactor tvm.testing.fixture fixtures that avoid redefining
+# outer variable names
+# pylint: disable=redefined-outer-name
Review Comment:
I'm not sure which 2 fixtures you mention here. Could you explain a little more. There were lots of `tvm.testing.fixture` functions defined and they all seemed to be interlinked, and I did not want to remove the `tvm.testing.fixture` calls for all of them, so I just disabled the linter.
##########
tests/python/contrib/test_hexagon/test_benchmark_elemwise_add.py:
##########
@@ -151,6 +151,7 @@ def main(a: T.handle, b: T.handle, c: T.handle):
for j in range(dim1_size):
C[i, j] = A[i, j] + B[i, j]
+ # pylint: enable=no-self-argument,invalid-name,missing-function-docstring
Review Comment:
Done, 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on a diff in pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
Mousius commented on code in PR #12082:
URL: https://github.com/apache/tvm/pull/12082#discussion_r923158579
##########
tests/python/contrib/test_hexagon/test_2d_physical_buffers.py:
##########
@@ -84,6 +83,12 @@ def target_host(target):
return tvm.target.Target(target, host=host)
+# Disabling redefined-outer-name for the whole file as there isn't any easy
+# solution yet to refactor tvm.testing.fixture fixtures that avoid redefining
+# outer variable names
+# pylint: disable=redefined-outer-name
Review Comment:
Does it not work if we just wrap the two fixtures?
##########
tests/python/contrib/test_hexagon/test_benchmark_elemwise_add.py:
##########
@@ -124,6 +119,7 @@ def _get_irmod_elemwise_add(
# Also: The VTCM budget is a very rough estimate, based only on experience.
# Assuming that it's even reasonable to use a hard-coded estimate AT ALL, this number
# may need tweaking.
+ # pylint: disable=unreachable
Review Comment:
Can we not just remove this and add it when it's functional?
##########
tests/python/contrib/test_hexagon/test_2d_physical_buffers.py:
##########
@@ -189,18 +196,19 @@ def schedule_args(
working_layout,
working_scope,
):
- InputTensor = te.placeholder(input_shape, dtype, name="Input")
- OutputTensor = te.compute(
- shape=InputTensor.shape,
- fcompute=lambda *indices: (2 * InputTensor[indices]).astype(dtype),
+ """Create and return the schedule and input args after applying layout transform"""
+ input_tensor = te.placeholder(input_shape, dtype, name="Input")
+ output_tensor = te.compute(
+ shape=input_tensor.shape,
+ fcompute=lambda *indices: (2 * input_tensor[indices]).astype(dtype),
name="Output",
)
- schedule = te.create_schedule(OutputTensor.op)
+ schedule = te.create_schedule(output_tensor.op)
- WriteCache = schedule.cache_write(OutputTensor, working_scope)
- ReadCache = schedule.cache_read(InputTensor, working_scope, [WriteCache])
+ write_cache = schedule.cache_write(output_tensor, working_scope)
+ read_cache = schedule.cache_read(input_tensor, working_scope, [write_cache])
- def apply_transform(tensor, layout):
+ def apply_transform(tensor, layout): # pylint: disable=inconsistent-return-statements
Review Comment:
Does this work with:
```
if layout == "nhwc":
return
```
or
```
if layout == "nhwc":
return None
```
instead?
##########
tests/python/contrib/test_hexagon/test_benchmark_elemwise_add.py:
##########
@@ -151,6 +151,7 @@ def main(a: T.handle, b: T.handle, c: T.handle):
for j in range(dim1_size):
C[i, j] = A[i, j] + B[i, j]
+ # pylint: enable=no-self-argument,invalid-name,missing-function-docstring
Review Comment:
```suggestion
# pylint: enable=no-self-argument,invalid-name,missing-function-docstring
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] quic-sanirudh commented on a diff in pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
quic-sanirudh commented on code in PR #12082:
URL: https://github.com/apache/tvm/pull/12082#discussion_r923627047
##########
tests/python/contrib/test_hexagon/test_2d_physical_buffers.py:
##########
@@ -189,18 +196,19 @@ def schedule_args(
working_layout,
working_scope,
):
- InputTensor = te.placeholder(input_shape, dtype, name="Input")
- OutputTensor = te.compute(
- shape=InputTensor.shape,
- fcompute=lambda *indices: (2 * InputTensor[indices]).astype(dtype),
+ """Create and return the schedule and input args after applying layout transform"""
+ input_tensor = te.placeholder(input_shape, dtype, name="Input")
+ output_tensor = te.compute(
+ shape=input_tensor.shape,
+ fcompute=lambda *indices: (2 * input_tensor[indices]).astype(dtype),
name="Output",
)
- schedule = te.create_schedule(OutputTensor.op)
+ schedule = te.create_schedule(output_tensor.op)
- WriteCache = schedule.cache_write(OutputTensor, working_scope)
- ReadCache = schedule.cache_read(InputTensor, working_scope, [WriteCache])
+ write_cache = schedule.cache_write(output_tensor, working_scope)
+ read_cache = schedule.cache_read(input_tensor, working_scope, [write_cache])
- def apply_transform(tensor, layout):
+ def apply_transform(tensor, layout): # pylint: disable=inconsistent-return-statements
Review Comment:
Thanks for catching this. I had meant to try out `return None` and had only temporarily disabled it, but forgot to get back to it. I've changed the code to `return None` now which works and which also forced me to get rid of the `elif` statements to `if` because they follow returns.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] quic-sanirudh commented on a diff in pull request #12082: [Pylint] Making hexagon tests pylint compliant Part 1 of N
Posted by GitBox <gi...@apache.org>.
quic-sanirudh commented on code in PR #12082:
URL: https://github.com/apache/tvm/pull/12082#discussion_r923625392
##########
tests/python/contrib/test_hexagon/test_benchmark_elemwise_add.py:
##########
@@ -124,6 +119,7 @@ def _get_irmod_elemwise_add(
# Also: The VTCM budget is a very rough estimate, based only on experience.
# Assuming that it's even reasonable to use a hard-coded estimate AT ALL, this number
# may need tweaking.
+ # pylint: disable=unreachable
Review Comment:
Yeah I wasn't sure if this needs to be available as the author seems to have left it intentionally unreachable, but I've commented it out for now and mentioned that it can be enabled when needed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org