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 2021/10/19 17:50:55 UTC
[GitHub] [tvm] grant-arm opened a new pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
grant-arm opened a new pull request #9323:
URL: https://github.com/apache/tvm/pull/9323
Change tvmc arguments to `-executor=aot -interface-api=c -unpacked-api=1` for Arm(R) Cortex(R)-M55 CPU and Arm(R) Ethos(TM)-U55 NPU Demo App.
https://github.com/apache/tvm/pull/9218/ changes the command line arguments required by tvmc which breaks the existing Arm(R) Cortex(R)-M55 CPU and Arm(R) Ethos(TM)-U55 NPU Demo App. This PR fixes the demo app.
--
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 change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734785216
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
Unsure where exactly this would go but I'd guess somewhere like `task_ci_setup.sh` ?
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734810451
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
Ah right, I misunderstood. I see what you're saying now.
@manupa-arm Any ideas?
--
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 change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734780326
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
+ cd apps/microtvm/ethosu
+ ./run_demo.sh --fvp_path /opt/arm/FVP_Corstone_SSE-300_Ethos-U55 --cmake_path /opt/arm/cmake/bin/cmake
+fi
+cd ../..
Review comment:
In fact, even with, doesn't this leave us in `apps` rather than resetting correctly?
--
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] manupa-arm commented on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948522912
Yes, however this PR passing CI does not give much assurance whether the demo is working or not.
What assurance would the user will have this is not broken again ?
Am I missing something or can't we atleast add something such as (if we want to explore other ways automating this later) :
https://github.com/apache/tvm/blob/88bf112454bbd30058719260e0fca9dd49c8692e/tests/scripts/task_cpp_unittest.sh#L43-L47
--
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] manupa-arm commented on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-946963653
I think we should enable running this in the CI -- to keep the demo up to date.
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734814235
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
@Mousius As an alternative, we could use ```python3 -m tvm.driver.tvmc``` instead of ```tvmc ...``` in ```run_demo.sh``` but I was reluctant to do that because I thought ```run_demo.sh``` should serve as an example to users in how to use ```tvmc``` from the command line.
--
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] grant-arm commented on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-949625690
I've added a CI test now that runs the Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app.
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734781948
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
@Mousius Could you please clarify where you think we should do 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 edited a comment on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius edited a comment on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948588096
@manupa-arm, to answer your points:
> Yes, however this PR passing CI does not give much assurance whether the demo is working or not.
I'm sure that @grant-arm would've tested this before raising a PR.
> What assurance would the user will have this is not broken again ?
That's what the follow up work would be, taking the time to triage it and put a solution in place.
My concern is for other users who are attempting to use this demo who may not be part of the TVM community and thus aren't accounted for, if we're happy to accept that they'll come to TVM and find it broken then we can wait until the automation question is resolved.
--
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] manupa-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734821111
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
Im fine with either, we can put a comment in the script to say that (one could use tvmc directly once Pip installed) or we can move it to bit higher position in terms of scripts.
Since this tested now, if we need more time to figure out where to put the installation, happy to take python3 -m tvm.driver.tvmc (with a comment explaining how it could be used alternatively)
--
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] manupa-arm commented on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-951604156
I think this is good enough for the fix (lets get this in) and thanks @grant-arm for quickly enabling it in the CI. I hope we can follow up with further changes.
--
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 change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r735610604
##########
File path: apps/microtvm/ethosu/run_demo.sh
##########
@@ -79,6 +83,30 @@ while (( $# )); do
fi
;;
+ --fvp_path)
+ if [ $# -gt 1 ]
+ then
+ export PATH="$2/models/Linux64_GCC-6.4:$PATH"
Review comment:
Doesn't that mean the default value for `FVP` should be `FVP_Corstone_SSE-300_Ethos-U55` and then overridden here? Creating a custom `PATH` inside the demo script seems overkill here.
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734808669
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] manupa-arm edited a comment on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm edited a comment on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948594982
Hi @Mousius ,
> My concern is for other users who are attempting to use this demo who may not be part of the TVM community and thus aren't accounted for, if we're happy to accept that they'll come to TVM and find it broken then we can wait until the automation question is resolved.
Merging this will not guarantee that there wont be such users. In same way this issue occurred, we could have the same problem tomorrow without us knowing. Its better to raise an issue and point to this PR to increase awareness, if thats what we are after.
Moreover, can you shed some light why the automation question is bigger piece of work that needs to be in a follow up ?
Maybe that might give right motivation that Im lacking here.
--
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 #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948502946
@manupa-arm it'd be better to land this fix which is effecting users (https://github.com/apache/tvm/pull/8922#issuecomment-946441386) and do any infrastructure work to automate this elsewhere.
--
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 change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734776847
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
Should this not be done earlier rather than as a side effect of invoking the unit test script?
##########
File path: apps/microtvm/ethosu/run_demo.sh
##########
@@ -79,6 +83,30 @@ while (( $# )); do
fi
;;
+ --fvp_path)
+ if [ $# -gt 1 ]
+ then
+ export PATH="$2/models/Linux64_GCC-6.4:$PATH"
Review comment:
Should this follow a similar pattern to the other arguments and export an `FVP_PATH` ?
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
Review comment:
It'd be good to put the path and pip check into a variable so it's clearer what this is checking for.
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
+ cd apps/microtvm/ethosu
+ ./run_demo.sh --fvp_path /opt/arm/FVP_Corstone_SSE-300_Ethos-U55 --cmake_path /opt/arm/cmake/bin/cmake
+fi
+cd ../..
Review comment:
If we don't `cd apps/microtvm/ethosu` then this will `cd ../..` out of the tvm directory - I don't think this is desired behaviour?
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
+ cd apps/microtvm/ethosu
+ ./run_demo.sh --fvp_path /opt/arm/FVP_Corstone_SSE-300_Ethos-U55 --cmake_path /opt/arm/cmake/bin/cmake
Review comment:
This could re-use the variable suggested above.
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734797871
##########
File path: apps/microtvm/ethosu/run_demo.sh
##########
@@ -79,6 +83,30 @@ while (( $# )); do
fi
;;
+ --fvp_path)
+ if [ $# -gt 1 ]
+ then
+ export PATH="$2/models/Linux64_GCC-6.4:$PATH"
Review comment:
I don't think that's what we want here.
The README for the demo app directs users to set PATH:
```export PATH=/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4:/opt/arm/cmake/bin:$PATH```
so that run_demo.sh can call ```FVP_Corstone_SSE-300_Ethos-U55...```
Providing an ```fvp_path``` argument simply sets PATH correctly so that we can continue to do that.
--
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] manupa-arm commented on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-951606263
Thanks @grant-arm @Mousius . Please do follow up with further concerns.
--
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] grant-arm commented on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948523954
I'll take a look at adding something to CI.
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734799137
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
I think this is fine for the current solution.
If we set a flag in future for use in multiple scripts/locations then we can think about setting it earlier.
--
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] manupa-arm commented on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948594982
Hi @Mousius ,
> My concern is for other users who are attempting to use this demo who may not be part of the TVM community and thus aren't accounted for, if we're happy to accept that they'll come to TVM and find it broken then we can wait until the automation question is resolved.
Merging this will not guarantee that there wont be such users. In same way this issue occurred, we could have the same problem tomorrow without us knowing. Its better to raise an issue and point to this PR to increase awareness, if thats what we are after.
Moreover, can you shed some line why the automation question is bigger piece of work that needs to be in a follow up ?
Maybe that might give right motivation that Im lacking here.
--
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 #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948588096
@manupa-arm, to answer your points:
> Yes, however this PR passing CI does not give much assurance whether the demo is working or not.
I'm sure that @grant-arm would've tested this before raising a PR.
> What assurance would the user will have this is not broken again ?
That's what the follow up work would be, taking the time to triage it and put a solution in place.
My concern is for other users who are attempting to use this demo who may not be part of the TVM community and thus aren't accounted for, if we're happy to accept that they'll come to TVM and find it broken then we can wait until the automation question is resolved.
--
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] grant-arm edited a comment on pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm edited a comment on pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#issuecomment-948523954
I'll take a look at adding something to CI as part of this PR.
--
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] manupa-arm merged pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
manupa-arm merged pull request #9323:
URL: https://github.com/apache/tvm/pull/9323
--
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 change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734801979
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
Review comment:
I'm not suggesting we shouldn't do it but this scripts responsibility is to run unit tests, this installs TVM python and all its dependencies as a side effect which I wouldn't expect from such a script.
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734808401
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
+ cd apps/microtvm/ethosu
+ ./run_demo.sh --fvp_path /opt/arm/FVP_Corstone_SSE-300_Ethos-U55 --cmake_path /opt/arm/cmake/bin/cmake
+fi
+cd ../..
Review comment:
Thanks @Mousius . Good catch!
--
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] grant-arm commented on a change in pull request #9323: Fix Arm(R) Ethos(TM)-U55 NPU Demo app
Posted by GitBox <gi...@apache.org>.
grant-arm commented on a change in pull request #9323:
URL: https://github.com/apache/tvm/pull/9323#discussion_r734808813
##########
File path: tests/scripts/task_cpp_unittest.sh
##########
@@ -45,3 +45,11 @@ cd apps/bundle_deploy
rm -rf build
make test_dynamic test_static
cd ../..
+
+# Test Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU demo app
+if test -f "/opt/arm/FVP_Corstone_SSE-300_Ethos-U55/models/Linux64_GCC-6.4/FVP_Corstone_SSE-300_Ethos-U55" && pip3 list | grep vela; then
+ sudo pip3 install -e python
+ cd apps/microtvm/ethosu
+ ./run_demo.sh --fvp_path /opt/arm/FVP_Corstone_SSE-300_Ethos-U55 --cmake_path /opt/arm/cmake/bin/cmake
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org