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

Enable torchvision #6439

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Enable torchvision #6439

wants to merge 3 commits into from

Conversation

ManfeiBai
Copy link
Collaborator

No description provided.

@ManfeiBai ManfeiBai changed the title Update Dockerfile Enable torchvision Feb 1, 2024
@ManfeiBai ManfeiBai marked this pull request as ready for review February 1, 2024 02:08
will-cromar
will-cromar reviewed Feb 1, 2024
View reviewed changes
Copy link
Collaborator

@will-cromar will-cromar left a comment

Choose a reason for hiding this comment

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

I think this is also going to overwrite the pytorch build in our nightly images, which is not what we want.

@vanbasten23
Copy link
Collaborator

vanbasten23 commented Feb 1, 2024 •
edited
Loading

I think this is also going to overwrite the pytorch build in our nightly images, which is not what we want.

Yeah, that's great point!
Perhaps we make the similar change in the release branch as r2.3, for example, so we get the correct torch package.
[Edit]: If the master branch code is used to create nightly image, perhaps we can install the official nightly wheels for example pip3 install --pre torch torchvision torchaudio --index-url https://download.pytorch.org/whl/nightly/cu121

@ManfeiBai
Copy link
Collaborator Author

we met the error RuntimeError: operator torchvision::nms does not exist in TPU CI too: https://github.com/pytorch/xla/runs/21133256024

@ManfeiBai @ysiraichi
@zpcore @cota @changm @wonjoolee95 @will-cromar @yeounoh @JackCaoG @alanwaketan @ezyang @qihqi
rebase (#6461)
beeda05 
Co-authored-by: Yukio Siraichi <[email protected]>
Co-authored-by: Pei Zhang <[email protected]>
Co-authored-by: Emilio Cota <[email protected]>
Co-authored-by: Mason Chang <[email protected]>
Co-authored-by: Wonjoo Lee <[email protected]>
Co-authored-by: Will Cromar <[email protected]>
Co-authored-by: Yeounoh Chung <[email protected]>
Co-authored-by: JackCaoG <[email protected]>
Co-authored-by: Jiewen Tan <[email protected]>
Co-authored-by: Edward Z. Yang <[email protected]>
Co-authored-by: qihqi <[email protected]>
@ManfeiBai
Copy link
Collaborator Author

ManfeiBai commented Feb 3, 2024 •
edited
Loading

TPU CI met the same error, so it happen on master branch too, the reason of this error looks like other tests use PyTorch/XLA built torchvision: https://github.com/pytorch/xla/actions/runs/7762948294/job/21175714067?pr=6439#logs, not the offical. torchvision wheel

we might want to modify to install torchvision in TPUCI too: pip_install --user --no-use-pep517 "git+https://github.com/pytorch/vision.git@$TORCHVISION_COMMIT"

same like this idea, we need to use torch/torchvision from source or built&then&use?

@will-cromar will-cromar mentioned this pull request Feb 5, 2024
Merged
@ManfeiBai
Copy link
Collaborator Author

since #6458 merged, close this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@will-cromar will-cromar will-cromar left review comments

@JackCaoG JackCaoG Awaiting requested review from JackCaoG

@yeounoh yeounoh Awaiting requested review from yeounoh

@mateuszlewko mateuszlewko Awaiting requested review from mateuszlewko

@stgpetrovic stgpetrovic Awaiting requested review from stgpetrovic

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ManfeiBai @vanbasten23 @will-cromar