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] Update CPU image to install PyTorch #14847

Merged
merged 5 commits into from
May 20, 2023
Merged

[CI] Update CPU image to install PyTorch #14847

merged 5 commits into from
May 20, 2023

Conversation

masahi
Copy link
Member

@masahi masahi commented May 13, 2023

Follow up to #14842 to actually update the image

@tvm-bot
Copy link
Collaborator

tvm-bot commented May 13, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@@ -19,7 +19,7 @@
[jenkins]
ci_arm: tlcpackstaging/ci_arm:20230504-142417-4d37a0a0
ci_cortexm: tlcpackstaging/ci_cortexm:20230504-142417-4d37a0a0
ci_cpu: tlcpackstaging/ci_cpu:20230504-142417-4d37a0a0
ci_cpu: tlcpackstaging/ci_cpu:20230513-200357-e54bbc73
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 noticed that all images are pointing to tlcpackstaging account rather than tlcpack since #14651. Should we fix them?

Copy link
Member

@yongwww yongwww May 13, 2023

Choose a reason for hiding this comment

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

looks the images were uploaded into https://hub.docker.com/u/tlcpack already, we can replace tlcpackstaging with tlcpack for the images with tag 20230504-142417-4d37a0a0. I can update it in follow-up pr after the cpu image with tag 20230513-200357-e54bbc73 is uploaded

Copy link
Member

Choose a reason for hiding this comment

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

cc: @driazati

Copy link
Member Author

@masahi masahi May 14, 2023

Choose a reason for hiding this comment

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

According to https://tvm.apache.org/docs/contribute/ci.html#updating-a-docker-image-tag, the current protocol is to reference tlcpack in a PR that updates an image, and CI automatically fetches the corresponding image from tlcpackstaging. The images are automatically uploaded to tlcpack after successful merge.

I updated the account to tlcpack.

Copy link
Member

@yongwww yongwww left a comment

Choose a reason for hiding this comment

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

LGTM

@masahi
Copy link
Member Author

masahi commented May 15, 2023

I don't get why we are still getting the free(): invalid pointer error even though I'm using --link-static and -DHIDE_PRIVATE_SYMBOLS=ON.

@yongwww
Copy link
Member

yongwww commented May 15, 2023

Previously what I did to debug the tests related to CI changes is to reproduce it locally first, the ec2 instance type for cpu container is r5.large.
Steps to reproduce:

  • . pull the docker image, and create a container from this image
  • . pull the tvm code in the container
  • . build tvm (generate the same config.cmake as ci: tests/scripts/task_config_build_cpu.sh build), set env variables like TVM_HOME

Then try to run the failed test, it should be running into same issue as ci reported

@masahi masahi merged commit 0dae360 into main May 20, 2023
@junrushao junrushao deleted the ci-cpu-pt branch May 26, 2023 16:58
mei-ye pushed a commit to mei-ye/tvm that referenced this pull request Jun 1, 2023
* [CI] Update CPU image to install PyTorch

* use link-static to prevent symbol conflict problem

* tlcpackstaging -> tlcpack

* disable tvmc pth tests for now

* fixed skip marker
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