Skip to content

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