Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CI] Separate out microTVM demo script #10003

Closed
wants to merge 2 commits into from
Closed

[CI] Separate out microTVM demo script #10003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
163644d
[CI] Separate out microTVM demo script
Mousius Jan 20, 2022
ec60121
Add missing -e to pip install
Mousius Jan 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,10 @@ stage('Build') {
script: "${docker_run} ${ci_qemu} ./tests/scripts/task_python_microtvm.sh",
label: 'Run microTVM tests',
)
sh (
script: "${docker_run} ${ci_qemu} ./tests/scripts/task_demo_microtvm.sh",
label: 'Run microTVM demos',
)
junit 'build/pytest-results/*.xml'
}
}
Expand Down
4 changes: 1 addition & 3 deletions apps/microtvm/ethosu/run_demo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ mobilenet_url='https://storage.googleapis.com/download.tensorflow.org/models/mob
curl --retry 64 -sSL ${mobilenet_url} | gunzip | tar -xvf - ./mobilenet_v1_1.0_224_quant.tflite

# Compile model for Arm(R) Cortex(R)-M55 CPU and Ethos(TM)-U55 NPU
# An alternative to using "python3 -m tvm.driver.tvmc" is to call
# "tvmc" directly once TVM has been pip installed.
python3 -m tvm.driver.tvmc compile --target="ethos-u -accelerator_config=ethos-u55-256, c" \
tvmc compile --target="ethos-u -accelerator_config=ethos-u55-256, c" \
--target-c-mcpu=cortex-m55 \
--runtime=crt \
--executor=aot \
Expand Down
3 changes: 3 additions & 0 deletions tests/scripts/task_ci_setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh apologies--i noticed the -e flag now. however, i still wonder if it's wise to create two import paths for tvm.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. we could do this, but we'd have to fix this 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."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

14 changes: 0 additions & 14 deletions tests/scripts/task_cpp_unittest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,3 @@ 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
FVP_PATH="/opt/arm/FVP_Corstone_SSE-300"

# TODO(@grant-arm): Remove once ci_cpu docker image has been updated to FVP_Corstone_SSE
if test ! -d $FVP_PATH; then
FVP_PATH="/opt/arm/FVP_Corstone_SSE-300_Ethos-U55"
fi

if test -d $FVP_PATH && pip3 list | grep vela; then
cd apps/microtvm/ethosu
./run_demo.sh --fvp_path $FVP_PATH --cmake_path /opt/arm/cmake/bin/cmake
cd ../../..
fi
23 changes: 23 additions & 0 deletions tests/scripts/task_demo_microtvm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/bin/bash
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

set -euxo pipefail

pushd apps/microtvm/ethosu
./run_demo.sh --cmake_path /opt/arm/cmake/bin/cmake
popd