You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "lhutton1 (via GitHub)" <gi...@apache.org> on 2023/08/08 08:24:44 UTC
[GitHub] [tvm] lhutton1 opened a new pull request, #15506: [Relay][Strategy] Use x86 pool schedules for arm_cpu
lhutton1 opened a new pull request, #15506:
URL: https://github.com/apache/tvm/pull/15506
Similar to https://github.com/apache/tvm/pull/15470, x86 schedules are used in place of generic schedules to improve performance.
Since the pooling strategy does not use `OpStrategy`, mocking is used to ensure the relevant `schedule_pool` function is called when lowing a Relay pooling operation with respect to a given target.
cc @ekalda @ashutosh-arm @neildhickey
--
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] ashutosh-arm commented on a diff in pull request #15506: [Relay][Strategy] Use x86 pool schedules for arm_cpu
Posted by "ashutosh-arm (via GitHub)" <gi...@apache.org>.
ashutosh-arm commented on code in PR #15506:
URL: https://github.com/apache/tvm/pull/15506#discussion_r1286837751
##########
tests/python/relay/strategy/test_select_implementation.py:
##########
@@ -187,5 +189,38 @@ def test_dense(target, expected_valid_impl, expected_impl):
assert selected_impl.name == expected_impl
+@pytest.mark.parametrize(
+ "target,schedule_func",
+ [
+ ("llvm -device=arm_cpu", topi.x86),
+ ("c -device=arm_cpu -mcpu=cortex-m55", topi.arm_cpu),
+ ],
+)
+def test_pool2d(target, schedule_func, monkeypatch):
+ target = tvm.target.Target(target)
+
+ data_shape = (1, 2, 2, 4)
+ dtype = "float32"
+
+ out = relay.nn.avg_pool2d(relay.var("data", shape=data_shape, dtype=dtype))
+ out = relay.Function(relay.analysis.free_vars(out), out)
+ out = tvm.IRModule.from_expr(out)
+
+ # Since pool does not use OpStrategy to determine the relevant schedule,
+ # we cannot simply check the schedule name that was selected with
+ # `select_implementation`. With this implementation of schedule selection,
+ # "pool.arm_cpu" will always be the schedule name, regardless of what schedule
+ # was selected. Instead, this test checks that the relevant schedule function
+ # is called when building the module.
+ mock_schedule = MagicMock()
+ mock_schedule.side_effect = lambda outs, layout: topi.generic.schedule_pool(outs, layout)
Review Comment:
question: `side_effect` is new to me. Does this work in the same way as described in the following places?
https://docs.pytest.org/en/7.1.x/how-to/monkeypatch.html#monkeypatching-functions
https://docs.python.org/3/library/unittest.mock-examples.html#side-effect-functions-and-iterables
Would it better to have a mock `schedule_pool()` within the test and check the value returned by it / prints done by it? This will make sure that the replacement actually happened only when x86 schedule was invoked.
##########
python/tvm/relay/op/strategy/arm_cpu.py:
##########
@@ -74,8 +74,7 @@ def schedule_pool_arm_cpu(attrs, outs, target):
and layout in ("NWC", "NHWC")
):
return topi.arm_cpu.schedule_pool(outs, layout)
- logger.warning("pool is not optimized for arm cpu.")
- return topi.generic.schedule_pool(outs, layout)
+ return topi.x86.schedule_pool(outs, layout)
Review Comment:
question: should we still wrap this under an `arm_cpu` schedule inside topi? In the future, if needed, the API can then further be optimized for the specific device.
--
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] lhutton1 commented on a diff in pull request #15506: [Relay][Strategy] Use x86 pool schedules for arm_cpu
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 commented on code in PR #15506:
URL: https://github.com/apache/tvm/pull/15506#discussion_r1286859213
##########
python/tvm/relay/op/strategy/arm_cpu.py:
##########
@@ -74,8 +74,7 @@ def schedule_pool_arm_cpu(attrs, outs, target):
and layout in ("NWC", "NHWC")
):
return topi.arm_cpu.schedule_pool(outs, layout)
- logger.warning("pool is not optimized for arm cpu.")
- return topi.generic.schedule_pool(outs, layout)
+ return topi.x86.schedule_pool(outs, layout)
Review Comment:
It's a good point, although I think we could do this as and when needed
##########
tests/python/relay/strategy/test_select_implementation.py:
##########
@@ -187,5 +189,38 @@ def test_dense(target, expected_valid_impl, expected_impl):
assert selected_impl.name == expected_impl
+@pytest.mark.parametrize(
+ "target,schedule_func",
+ [
+ ("llvm -device=arm_cpu", topi.x86),
+ ("c -device=arm_cpu -mcpu=cortex-m55", topi.arm_cpu),
+ ],
+)
+def test_pool2d(target, schedule_func, monkeypatch):
+ target = tvm.target.Target(target)
+
+ data_shape = (1, 2, 2, 4)
+ dtype = "float32"
+
+ out = relay.nn.avg_pool2d(relay.var("data", shape=data_shape, dtype=dtype))
+ out = relay.Function(relay.analysis.free_vars(out), out)
+ out = tvm.IRModule.from_expr(out)
+
+ # Since pool does not use OpStrategy to determine the relevant schedule,
+ # we cannot simply check the schedule name that was selected with
+ # `select_implementation`. With this implementation of schedule selection,
+ # "pool.arm_cpu" will always be the schedule name, regardless of what schedule
+ # was selected. Instead, this test checks that the relevant schedule function
+ # is called when building the module.
+ mock_schedule = MagicMock()
+ mock_schedule.side_effect = lambda outs, layout: topi.generic.schedule_pool(outs, layout)
Review Comment:
Yes essentially it was setting a return value for the mocked function.
The return type of `schedule_pool` is a schedule. I took a look at the object but it doesn't really offer anything indicative of an x86 schedule having been used. Also, I don't believe `schedule` can be equality checked very easily. Therefore, I think the simplest way is to check whether the expected function was called.
You did give me an idea that the test could be simplified a little. Rather than check from `relay.build` we can simply check from `schedule_pool_arm_cpu`. This will remove the side-effect nastiness.
--
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] lhutton1 commented on pull request #15506: [Relay][Strategy] Use x86 pool schedules for arm_cpu
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 commented on PR #15506:
URL: https://github.com/apache/tvm/pull/15506#issuecomment-1670916256
Thanks @ashutosh-arm!
--
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] lhutton1 merged pull request #15506: [Relay][Strategy] Use x86 pool schedules for arm_cpu
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 merged PR #15506:
URL: https://github.com/apache/tvm/pull/15506
--
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] tvm-bot commented on pull request #15506: [Relay][Strategy] Use x86 pool schedules for arm_cpu
Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #15506:
URL: https://github.com/apache/tvm/pull/15506#issuecomment-1669149102
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @shingjan <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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