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/04/15 21:44:12 UTC
[GitHub] [tvm] mehrdadh opened a new pull request, #11026: [Hexagon] Refactor test scripts
mehrdadh opened a new pull request, #11026:
URL: https://github.com/apache/tvm/pull/11026
This PR refactors hexagon test bash script. This will avoid running hexagon tests twice in CI.
cc @driazati @Lunderberg
--
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] areusch merged pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
areusch merged PR #11026:
URL: https://github.com/apache/tvm/pull/11026
--
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] kparzysz-quic commented on pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #11026:
URL: https://github.com/apache/tvm/pull/11026#issuecomment-1100438270
LGTM
--
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] Lunderberg commented on a diff in pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on code in PR #11026:
URL: https://github.com/apache/tvm/pull/11026#discussion_r851578524
##########
tests/scripts/task_python_hexagon.sh:
##########
@@ -18,10 +18,34 @@
set -e
set -u
-set -x
-source tests/scripts/setup-pytest-env.sh
+device_serial="simulator"
+if [ $# -ge 1 ] && [[ "$1" = "--device" ]]; then
+ shift 1
+ device_serial="$1"
+ shift
+fi
+source tests/scripts/setup-pytest-env.sh
make cython3
+if [[ "${device_serial}" == "simulator" ]]; then
+ export TVM_TRACKER_PORT=9190
+ export TVM_TRACKER_HOST=0.0.0.0
+ env PYTHONPATH=python python3 -m tvm.exec.rpc_tracker --host "${TVM_TRACKER_HOST}" --port "${TVM_TRACKER_PORT}" &
Review Comment:
The bash way to ensure it gets cleaned up would be to use `trap my_cleanup_function EXIT` to run a function when the script exits. That said, I like to idea of having it all in python, for uniformity.
--
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] driazati commented on a diff in pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #11026:
URL: https://github.com/apache/tvm/pull/11026#discussion_r851546952
##########
tests/scripts/task_python_hexagon.sh:
##########
@@ -18,10 +18,34 @@
set -e
set -u
-set -x
-source tests/scripts/setup-pytest-env.sh
+device_serial="simulator"
+if [ $# -ge 1 ] && [[ "$1" = "--device" ]]; then
+ shift 1
+ device_serial="$1"
+ shift
+fi
+source tests/scripts/setup-pytest-env.sh
make cython3
+if [[ "${device_serial}" == "simulator" ]]; then
+ export TVM_TRACKER_PORT=9190
+ export TVM_TRACKER_HOST=0.0.0.0
+ env PYTHONPATH=python python3 -m tvm.exec.rpc_tracker --host "${TVM_TRACKER_HOST}" --port "${TVM_TRACKER_PORT}" &
Review Comment:
I’m not good enough at bash to know off the top of my head, but does this Python process exit when the script is done even if there is an error before the `kill` below?
--
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] mehrdadh commented on pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #11026:
URL: https://github.com/apache/tvm/pull/11026#issuecomment-1100427732
cc @kparzysz-quic
--
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] mehrdadh commented on a diff in pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11026:
URL: https://github.com/apache/tvm/pull/11026#discussion_r852216692
##########
tests/scripts/task_python_hexagon.sh:
##########
@@ -18,10 +18,34 @@
set -e
set -u
-set -x
-source tests/scripts/setup-pytest-env.sh
+device_serial="simulator"
+if [ $# -ge 1 ] && [[ "$1" = "--device" ]]; then
+ shift 1
+ device_serial="$1"
+ shift
+fi
+source tests/scripts/setup-pytest-env.sh
make cython3
+if [[ "${device_serial}" == "simulator" ]]; then
+ export TVM_TRACKER_PORT=9190
+ export TVM_TRACKER_HOST=0.0.0.0
+ env PYTHONPATH=python python3 -m tvm.exec.rpc_tracker --host "${TVM_TRACKER_HOST}" --port "${TVM_TRACKER_PORT}" &
Review Comment:
Thanks for the feedbacks. We could fix this in a follow up 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] mehrdadh commented on a diff in pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #11026:
URL: https://github.com/apache/tvm/pull/11026#discussion_r851555881
##########
tests/scripts/task_python_hexagon.sh:
##########
@@ -18,10 +18,34 @@
set -e
set -u
-set -x
-source tests/scripts/setup-pytest-env.sh
+device_serial="simulator"
+if [ $# -ge 1 ] && [[ "$1" = "--device" ]]; then
+ shift 1
+ device_serial="$1"
+ shift
+fi
+source tests/scripts/setup-pytest-env.sh
make cython3
+if [[ "${device_serial}" == "simulator" ]]; then
+ export TVM_TRACKER_PORT=9190
+ export TVM_TRACKER_HOST=0.0.0.0
+ env PYTHONPATH=python python3 -m tvm.exec.rpc_tracker --host "${TVM_TRACKER_HOST}" --port "${TVM_TRACKER_PORT}" &
Review Comment:
probably not, so I guess whether the script reaches the `kill` command depends on the brhaviour of `run_pytest` command in case an error happens on one of the tests.
I think there are other ways to run tracker in python test files which we can replace them.
##########
tests/scripts/task_python_hexagon.sh:
##########
@@ -18,10 +18,34 @@
set -e
set -u
-set -x
-source tests/scripts/setup-pytest-env.sh
+device_serial="simulator"
+if [ $# -ge 1 ] && [[ "$1" = "--device" ]]; then
+ shift 1
+ device_serial="$1"
+ shift
+fi
+source tests/scripts/setup-pytest-env.sh
make cython3
+if [[ "${device_serial}" == "simulator" ]]; then
+ export TVM_TRACKER_PORT=9190
+ export TVM_TRACKER_HOST=0.0.0.0
+ env PYTHONPATH=python python3 -m tvm.exec.rpc_tracker --host "${TVM_TRACKER_HOST}" --port "${TVM_TRACKER_PORT}" &
Review Comment:
probably not, so I guess whether the script reaches the `kill` command depends on the brhaviour of `run_pytest` command in case an error happens on one of the tests.
I think there are other ways to run tracker in python test files which we can replace it.
--
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] mehrdadh commented on pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on PR #11026:
URL: https://github.com/apache/tvm/pull/11026#issuecomment-1101518725
@driazati looks like the CI is buster because it is using old Jenkinsfile. How can I fix 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] driazati commented on pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
driazati commented on PR #11026:
URL: https://github.com/apache/tvm/pull/11026#issuecomment-1101568070
You’ll need to close this PR and make a new one from the branch under apache/tvm instead of the one in your tvm fork (Jenkins doesn’t read the Jenkinsfile from forked repos for security reasons)
--
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] kparzysz-quic commented on a diff in pull request #11026: [Hexagon] Refactor test scripts
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #11026:
URL: https://github.com/apache/tvm/pull/11026#discussion_r851633432
##########
tests/scripts/task_python_hexagon.sh:
##########
@@ -18,10 +18,34 @@
set -e
set -u
-set -x
-source tests/scripts/setup-pytest-env.sh
+device_serial="simulator"
+if [ $# -ge 1 ] && [[ "$1" = "--device" ]]; then
+ shift 1
+ device_serial="$1"
+ shift
+fi
+source tests/scripts/setup-pytest-env.sh
make cython3
+if [[ "${device_serial}" == "simulator" ]]; then
+ export TVM_TRACKER_PORT=9190
+ export TVM_TRACKER_HOST=0.0.0.0
+ env PYTHONPATH=python python3 -m tvm.exec.rpc_tracker --host "${TVM_TRACKER_HOST}" --port "${TVM_TRACKER_PORT}" &
Review Comment:
This all runs in docker in CI, so from that perspective it doesn't matter if the script leaves some processes behind.
We were hitting this issue in our downstream CI and we start the tracker in a separate process group, then terminate all processes from the group with a single `kill -TERM -${PGID}` command. I don't know what your approach to running tracker from python would be, but keep in mind that python is not very good at guaranteeing global cleanup: if the program exits in some abnormal way (say, due to a syntax error), the execution will just stop and no "destructors" will be run.
In any case, this issue was present even before this refactoring, so this is not a regression. I'm ok with checking this in, and addressing the cleanup issue in another 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