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.

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

[CI] Set USE_CMSISNN and USE_ETHOSU off in task_config_build_cpu.sh #12456

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

ekalda
Copy link
Contributor

@ekalda ekalda commented Aug 16, 2022

The dependencies for these have moved into ci_cortexm Docker
image, so there is not much point in building them for ci_cpu as we
can't run the associated tests.

cc @Mousius @areusch @driazati @gigiblender

@ekalda
Copy link
Contributor Author

ekalda commented Aug 16, 2022

cc @leandron

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

Yes, the flags here are not tested in a context of ci_cpu, and are covered by the https://github.com/apache/tvm/blob/main/tests/scripts/task_config_build_cortexm.sh setup, where they are properly tested.

LGTM

@areusch
Copy link
Contributor

areusch commented Aug 18, 2022

thanks @ekalda ! looks like we are still running one of those tests in CPU somehow, maybe need to make sure that one got moved to arm and then remove it from CPU.

@ekalda
Copy link
Contributor Author

ekalda commented Aug 19, 2022

Hi @areusch , sorry I've had very busy last few days, but I'm on it now - I looked into this and the problem is that we still install Vela in ci_cpu (https://github.com/apache/tvm/blob/main/docker/Dockerfile.ci_cpu#L129) despite moving Ethos-U into ci_cortexm and we use the presence of Vela as a proxy when deciding whether to run a test or not if it is outside of test_ethosu folder. As a result of this, there are some Ethos-U based tests in tvmc testing which are failing because pytest.importorskip("ethosu.vela") can't do it's job (https://github.com/apache/tvm/blob/main/tests/python/driver/tvmc/test_compiler.py#L456) .

So I think I'll park this change for a bit and propose a docker image change and once that is done, we should be able to merge this twoliner patch :)

@ekalda ekalda mentioned this pull request Aug 24, 2022
7 tasks
@ekalda
Copy link
Contributor Author

ekalda commented Sep 12, 2022

@tvm-bot rerun

The dependencies for these have moved into ci_cortexm Docker
image, so there is not much point in building them for ci_cpu as we
can't run the associated tests.
@ekalda
Copy link
Contributor Author

ekalda commented Sep 13, 2022

@driazati @areusch @leandron it's passing the CI now after the upgrade to the Docker image, so should be fine to merge.

@driazati driazati merged commit a0cbefb into apache:main Sep 14, 2022
@ekalda ekalda deleted the task-config-build-cpu branch September 14, 2022 16:29
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
…pache#12456)

The dependencies for these have moved into ci_cortexm Docker
image, so there is not much point in building them for ci_cpu as we
can't run the associated tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants