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/01/20 15:39:24 UTC
[GitHub] [tvm] Mousius opened a new pull request #10003: [CI] Separate out microTVM demo script
Mousius opened a new pull request #10003:
URL: https://github.com/apache/tvm/pull/10003
The initial solution to running the demo in CI was to include it as part of `task_cpp_unittest.sh` - this patch aims to clean up the CI scripts to run the demo so it's ran as part of a clear script and uses `tvmc` as a user would.
--
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 #10003: [CI] Separate out microTVM demo script
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #10003:
URL: https://github.com/apache/tvm/pull/10003#discussion_r798466565
##########
File path: tests/scripts/task_ci_setup.sh
##########
@@ -39,3 +39,6 @@ python3 -m pip install --user tlcpack-sphinx-addon==0.2.1 synr==0.6.0
# Ensure no stale pytest-results remain from a previous test run.
(cd build && rm -rf pytest-results)
+
+# Install tvm as a local package to allow tvmc to work
+python3 -m pip install --user -e ./python
Review comment:
I don't think the Project API addresses this need, as these demos represent a user with an existing application and integration with just the model rather than generating a full project they want to orchestrate from TVM. Porting to the Project API would then be unfamiliar territory to those users? As such I think we can just create a virtual env to use for running the demos in?
--
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 commented on a change in pull request #10003: [CI] Separate out microTVM demo script
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10003:
URL: https://github.com/apache/tvm/pull/10003#discussion_r789176197
##########
File path: tests/scripts/task_ci_setup.sh
##########
@@ -39,3 +39,6 @@ python3 -m pip install --user tlcpack-sphinx-addon==0.2.1 synr==0.6.0
# Ensure no stale pytest-results remain from a previous test run.
(cd build && rm -rf pytest-results)
+
+# Install tvm as a local package to allow tvmc to work
+python3 -m pip install --user -e ./python
Review comment:
i think this would install tvm in `.local`, i wonder if it would cause some staleness issues for folks running this script locally and invoking `import tvm` (e.g. would it copy all the .pyc files into `~/.local/lib/pythonXX/tvm`)? maybe a better strategy is to `python3 -mtvm.driver.tvmc` in `run_demo.sh` above, and just comment that this way is used to avoid these problems?
--
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 commented on a change in pull request #10003: [CI] Separate out microTVM demo script
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10003:
URL: https://github.com/apache/tvm/pull/10003#discussion_r797881458
##########
File path: tests/scripts/task_ci_setup.sh
##########
@@ -39,3 +39,6 @@ python3 -m pip install --user tlcpack-sphinx-addon==0.2.1 synr==0.6.0
# Ensure no stale pytest-results remain from a previous test run.
(cd build && rm -rf pytest-results)
+
+# Install tvm as a local package to allow tvmc to work
+python3 -m pip install --user -e ./python
Review comment:
hmm. we could do this, but we'd have to fix [this](https://github.com/apache/tvm/blob/main/docker/bash.sh#L396) hack at least for the purposes of running the demo. I guess the best way would be to remove the PYTHONPATH override in `task_demo_microtvm.sh`.
i agree we need a venv to accurately reproduce the real-world envrionment where TVM is installed. our tests should also all probably run against that venv rather than with the PYTHONPATH hack. it seems like for this PR, that would be sufficent.
Project API kind of tries to address this as well--to separate the Python deps needed to interact with a platform from those needed for TVM. we could solve this problem once for apps/microtvm projects and port this to use Project API, but that does come with, well, porting this to Project API. and, there's more to that, as there would need to be some way to specify "use tvm, but the version being tested in CI, not the one from pip."
--
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 commented on a change in pull request #10003: [CI] Separate out microTVM demo script
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10003:
URL: https://github.com/apache/tvm/pull/10003#discussion_r798796890
##########
File path: tests/scripts/task_ci_setup.sh
##########
@@ -39,3 +39,6 @@ python3 -m pip install --user tlcpack-sphinx-addon==0.2.1 synr==0.6.0
# Ensure no stale pytest-results remain from a previous test run.
(cd build && rm -rf pytest-results)
+
+# Install tvm as a local package to allow tvmc to work
+python3 -m pip install --user -e ./python
Review comment:
yeah the thing i wanted to point out is that when there is a PYTHONPATH override, the venv may not be effective. you'll at least need to remove that override.
if you go this route (venv + explicitly clear PYTHONPATH for this one test), can we file a cleanup issue to run all of the pytests in a venv with TVM properly installed using pip?
--
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 commented on a change in pull request #10003: [CI] Separate out microTVM demo script
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10003:
URL: https://github.com/apache/tvm/pull/10003#discussion_r789176505
##########
File path: tests/scripts/task_ci_setup.sh
##########
@@ -39,3 +39,6 @@ python3 -m pip install --user tlcpack-sphinx-addon==0.2.1 synr==0.6.0
# Ensure no stale pytest-results remain from a previous test run.
(cd build && rm -rf pytest-results)
+
+# Install tvm as a local package to allow tvmc to work
+python3 -m pip install --user -e ./python
Review comment:
oh apologies--i noticed the `-e` flag now. however, i still wonder if it's wise to create two import paths for tvm.
--
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 #10003: [CI] Separate out microTVM demo script
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #10003:
URL: https://github.com/apache/tvm/pull/10003#discussion_r789589104
##########
File path: tests/scripts/task_ci_setup.sh
##########
@@ -39,3 +39,6 @@ python3 -m pip install --user tlcpack-sphinx-addon==0.2.1 synr==0.6.0
# Ensure no stale pytest-results remain from a previous test run.
(cd build && rm -rf pytest-results)
+
+# Install tvm as a local package to allow tvmc to work
+python3 -m pip install --user -e ./python
Review comment:
> maybe a better strategy is to python3 -mtvm.driver.tvmc in run_demo.sh above, and just comment that this way is used to avoid these problems?
This is what we currently have and what I'm hoping to fix as we shouldn't be exposing any implementation detail of TVM to a user as part of running a tutorial or demo. Given this is ran in `task_ci_setup.sh`, shouldn't this only be ran as part of CI and we should fix CI not using a clean environment if that's a concern?
Another potential way is we can hide it with something like `alias tvmc="python3 -mtvm.driver.tvmc"` in the CI script which at least hides the CI setup from the user? Thoughts on that approach @areusch ?
--
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