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/11/14 13:46:48 UTC
[GitHub] [tvm] d-smirnov opened a new pull request, #13369: [usmp] Hill Climb greedy layout size check relaxed
d-smirnov opened a new pull request, #13369:
URL: https://github.com/apache/tvm/pull/13369
This PR relaxes the size check for "initial" layout as the later permutations could lead to winning combination.
--
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 #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13369:
URL: https://github.com/apache/tvm/pull/13369#issuecomment-1313736477
<!---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-->
* No users to tag found in teams: `usmp` <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
[GitHub] [tvm] leandron merged pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
leandron merged PR #13369:
URL: https://github.com/apache/tvm/pull/13369
--
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] Alexey-Yazev commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
Alexey-Yazev commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1023569114
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -142,18 +141,8 @@ def test_networks_with_usmp_and_cascader_wo_striping(accel_type, model_url, work
@pytest.mark.parametrize(
"accel_type, model_url, workspace_size",
[
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
+ ("ethos-u55-256", MOBILENET_V1_URL, 1005312),
+ ("ethos-u55-256", MOBILENET_V2_URL, 1162368),
Review Comment:
Maybe revert to old values in test since cascade scheduling is unstable, I've prepared a fix for 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] leandron commented on pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
leandron commented on PR #13369:
URL: https://github.com/apache/tvm/pull/13369#issuecomment-1313867387
cc @Alex-grovety @sergey-grovety @ilyag-grovety for reviews
--
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] d-smirnov commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
d-smirnov commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1025073170
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -142,18 +141,8 @@ def test_networks_with_usmp_and_cascader_wo_striping(accel_type, model_url, work
@pytest.mark.parametrize(
"accel_type, model_url, workspace_size",
[
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
+ ("ethos-u55-256", MOBILENET_V1_URL, 1005312),
+ ("ethos-u55-256", MOBILENET_V2_URL, 1162368),
Review Comment:
Thanks. Added extra check
--
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] Alexey-Yazev commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
Alexey-Yazev commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1025005038
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -142,18 +141,8 @@ def test_networks_with_usmp_and_cascader_wo_striping(accel_type, model_url, work
@pytest.mark.parametrize(
"accel_type, model_url, workspace_size",
[
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
+ ("ethos-u55-256", MOBILENET_V1_URL, 1005312),
+ ("ethos-u55-256", MOBILENET_V2_URL, 1162368),
Review Comment:
As I see the test_networks.py passed, there are problems with the test_codegen.py. There are cases when buffer_info_arr (in HillClimbAllocator::PlanMemory) is empty, I suggest adding a size check for buffer_info_arr and return empty map in this case.
--
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] d-smirnov commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
d-smirnov commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1024613292
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -142,18 +141,8 @@ def test_networks_with_usmp_and_cascader_wo_striping(accel_type, model_url, work
@pytest.mark.parametrize(
"accel_type, model_url, workspace_size",
[
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
+ ("ethos-u55-256", MOBILENET_V1_URL, 1005312),
+ ("ethos-u55-256", MOBILENET_V2_URL, 1162368),
Review Comment:
Looks like I can confirm instability with the cascade scheduling as in two subsequent runs of `tests/python/contrib/test_ethosu/test_networks.py::test_networks_with_usmp_and_cascader_with_striping[ethos-u55-256-model_url1-...` the _hill_climb_ allocator was supplied at first run with 8 and, at second, with 7 buffers to allocate, which resulted in different allocated sizes (1374688 and 1162448 bytes correspondingly, in both cases all the buffers "conflicting" with each other).
--
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] d-smirnov commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
d-smirnov commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1022686177
##########
src/tir/usmp/algo/hill_climb.cc:
##########
@@ -312,6 +331,14 @@ class HillClimbAllocator : public GreedyBase {
Map<BufferInfo, PoolAllocation> result;
// return winning combination
for (auto it : result_pool_allocations) {
+ // post-check that everything was fit
+ const BufferInfoNode* buf = it.first;
+ const PoolAllocation& pa = it.second;
+ if (NullValue<PoolInfo>().same_as(pa->pool_info) ||
+ !IsValidPlacement(pa->pool_info, pa->byte_offset->value, buf->size_bytes->value)) {
+ std::unordered_map<PoolInfo, size_t, ObjectPtrHash, ObjectPtrEqual> m = {};
+ SelectPlacementPool(GetRef<BufferInfo>(buf), m);
Review Comment:
The "optimistic" estimation can be calculated from current error message as sum of `size_bytes` (buffer size) and `size_hint_bytes` (pool size)
--
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] Alexey-Yazev commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
Alexey-Yazev commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1022458150
##########
src/tir/usmp/algo/hill_climb.cc:
##########
@@ -312,6 +331,14 @@ class HillClimbAllocator : public GreedyBase {
Map<BufferInfo, PoolAllocation> result;
// return winning combination
for (auto it : result_pool_allocations) {
+ // post-check that everything was fit
+ const BufferInfoNode* buf = it.first;
+ const PoolAllocation& pa = it.second;
+ if (NullValue<PoolInfo>().same_as(pa->pool_info) ||
+ !IsValidPlacement(pa->pool_info, pa->byte_offset->value, buf->size_bytes->value)) {
+ std::unordered_map<PoolInfo, size_t, ObjectPtrHash, ObjectPtrEqual> m = {};
+ SelectPlacementPool(GetRef<BufferInfo>(buf), m);
Review Comment:
maybe add an error message specifying the required memory size instead of the error message from SelectPlacementPool?
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -20,7 +20,7 @@
pytest.importorskip("ethosu.vela")
import numpy as np
-
+import tvm
Review Comment:
on the 27th line the same import remained
--
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] d-smirnov commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
d-smirnov commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1022687154
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -20,7 +20,7 @@
pytest.importorskip("ethosu.vela")
import numpy as np
-
+import tvm
Review Comment:
removed
--
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] d-smirnov commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
d-smirnov commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1024613292
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -142,18 +141,8 @@ def test_networks_with_usmp_and_cascader_wo_striping(accel_type, model_url, work
@pytest.mark.parametrize(
"accel_type, model_url, workspace_size",
[
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
+ ("ethos-u55-256", MOBILENET_V1_URL, 1005312),
+ ("ethos-u55-256", MOBILENET_V2_URL, 1162368),
Review Comment:
Looks like I can confirm instability with the cascade scheduling as in two subsequent runs of `tests/python/contrib/test_ethosu/test_networks.py::test_networks_with_usmp_and_cascader_with_striping[ethos-u55-256-model_url1-...` the _hill_climb_ allocator was supplied at first run with 8 and, at second, with 7 buffers to allocate, which resulted in different allocated sizes (1374688 and 1162448 bytes correspondingly, in both cases all the buffers were "conflicting" with each other).
--
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] d-smirnov commented on a diff in pull request #13369: [usmp] Hill Climb greedy layout size check relaxed
Posted by GitBox <gi...@apache.org>.
d-smirnov commented on code in PR #13369:
URL: https://github.com/apache/tvm/pull/13369#discussion_r1024840349
##########
tests/python/contrib/test_ethosu/test_networks.py:
##########
@@ -142,18 +141,8 @@ def test_networks_with_usmp_and_cascader_wo_striping(accel_type, model_url, work
@pytest.mark.parametrize(
"accel_type, model_url, workspace_size",
[
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- ("ethos-u55-256", MOBILENET_V1_URL, 1010000),
- # Checks the same test case multiple times to make sure its not flaky
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
- ("ethos-u55-256", MOBILENET_V2_URL, 1400000),
+ ("ethos-u55-256", MOBILENET_V1_URL, 1005312),
+ ("ethos-u55-256", MOBILENET_V2_URL, 1162368),
Review Comment:
url1 = MOBILENET_V2_URL
@Alexey-Yazev please advise with the pool size values as currently all Cortex-M tests failed
--
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