Skip to content

Conversation

@u99127
Copy link

@u99127 u99127 commented Dec 17, 2020

Add testing for ACL to the CI for AArch64. A PR follows
to add this to the Jenkinsfile once the docker changes land.
We also need a separate script to run the tests as the full
python integration tests are currently broken.

[I've tried this on an AArch64 Linux machine and it appears to
work ok.]

I think we'll need to bake a new docker image before the
jenkins file changes land.

@zhiics @comaniac @lhutton1

Add testing for ACL to the CI for AArch64. A PR follows
to add this to the Jenkinsfile once the docker changes land.

We also need a separate script to run the tests as the full
integration tests are currently broken.
@comaniac comaniac changed the title Add ACL testing to the CI for AArch64. [CI] Add ACL to the CI for AArch64 Dec 17, 2020
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM to the dockerfile and build config. I expect the follow-up PR would also enable USE_ARM_COMPUTE_LIB_GRAPH_RUNTIME to cover ACL runtime unit tests.

On the other hand, I don't think we need to add a new task script for ACL. The current Jenkinsfile already covers the ACL unit test in L280 (currently comment out). Specifically, task_python_integration.sh covers all the unit tests under tests/python/contrib, including ACL tests. As a result, I would prefer to bring L280 back in the Jenkinsfile to enable all the tests.

Note: Docker image has to be updated after this PR. cc @tqchen

@u99127
Copy link
Author

u99127 commented Dec 17, 2020

LGTM to the dockerfile and build config. I expect the follow-up PR would also enable USE_ARM_COMPUTE_LIB_GRAPH_RUNTIME to cover ACL runtime unit tests.

That is the next step once this is working .Turning that on probably also needs a bit of tweaking in the infra for the tests which I'll try and fix up next. The idea was to get the docker changes in and the testing on par with what happens with the other pipeline but natively.

On the other hand, I don't think we need to add a new task script for ACL. The current Jenkinsfile already covers the ACL unit test in L280 (currently comment out). Specifically, task_python_integration.sh covers all the unit tests under tests/python/contrib, including ACL tests. As a result, I would prefer to bring L280 back in the Jenkinsfile to enable all the tests.

As I indicated in the covering letter, turning on task_python_integration.sh was failing in my test run with failures with tolerances that I suspect will need more time than I have this side of the holidays. It also means we can get quicker to first getting ACL runtime tests in the CI first.

Note: Docker image has to be updated after this PR. cc @tqchen

regards
Ramana

@comaniac
Copy link
Contributor

That is the next step once this is working .Turning that on probably also needs a bit of tweaking in the infra for the tests which I'll try and fix up next. The idea was to get the docker changes in and the testing on par with what happens with the other pipeline but natively.

I see. Thanks for the clarification.

As I indicated in the covering letter, turning on task_python_integration.sh was failing in my test run with failures with tolerances that I suspect will need more time than I have this side of the holidays. It also means we can get quicker to first getting ACL runtime tests in the CI first.

I must be missing the description before. Then it makes sense to me to have a separate script first until task_python_integration.sh fully works on AArch64 platform.

@u99127
Copy link
Author

u99127 commented Dec 17, 2020

As I indicated in the covering letter, turning on task_python_integration.sh was failing in my test run with failures with tolerances that I suspect will need more time than I have this side of the holidays. It also means we can get quicker to first getting ACL runtime tests in the CI first.

I must be missing the description before. Then it makes sense to me to have a separate script first until task_python_integration.sh fully works on AArch64 platform.

Thanks for the swift review. Yes , sorry I don't think I was explicit enough in saying that.

Ramana

@comaniac comaniac merged commit 6b9b898 into apache:main Dec 18, 2020
@comaniac
Copy link
Contributor

Thanks @u99127 @zhiics.

masahi pushed a commit to masahi/tvm that referenced this pull request Dec 18, 2020
Add testing for ACL to the CI for AArch64. A PR follows
to add this to the Jenkinsfile once the docker changes land.

We also need a separate script to run the tests as the full
integration tests are currently broken.
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Jan 20, 2021
Add testing for ACL to the CI for AArch64. A PR follows
to add this to the Jenkinsfile once the docker changes land.

We also need a separate script to run the tests as the full
integration tests are currently broken.
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jan 21, 2021
Add testing for ACL to the CI for AArch64. A PR follows
to add this to the Jenkinsfile once the docker changes land.

We also need a separate script to run the tests as the full
integration tests are currently broken.
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
Add testing for ACL to the CI for AArch64. A PR follows
to add this to the Jenkinsfile once the docker changes land.

We also need a separate script to run the tests as the full
integration tests are currently broken.
@u99127
Copy link
Author

u99127 commented Feb 24, 2021

Need a docker image update for ci_arm - @tqchen @zhiics , could this be done please ?

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.

3 participants