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

move 2 binaries to S3 #841

Merged
merged 4 commits into from
Sep 8, 2021
Merged

move 2 binaries to S3 #841

merged 4 commits into from
Sep 8, 2021

Conversation

mszhanyi
Copy link
Contributor

@mszhanyi mszhanyi commented Sep 2, 2021

Fix #788

@mszhanyi
Copy link
Contributor Author

mszhanyi commented Sep 2, 2021

@seemethere @peterjc123

Copy link
Contributor

@peterjc123 peterjc123 left a comment

Choose a reason for hiding this comment

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

@mszhanyi
Copy link
Contributor Author

mszhanyi commented Sep 3, 2021

Please see my comment. https://github.com/pytorch/vision/pull/4058/files#r701067987

@peterjc123 , that's a different issue. Time is a little long, I don't remember very clearly why the snapshot is in that place.
I first find the version conflict issue is in pytorch/vision#4058 (comment), which broke the vision build. With cuda10.2, there's always No CUDA Runtime is found,... when running python setup.py install.
For Pytorch build, it might be OK because Pytorch build is in CPU machine. But vision build is in GPU machine, I think the reason is vision build & test is in one workflow.

@malfet
Copy link
Contributor

malfet commented Sep 3, 2021

I think PR title is a bit misleading: It does not move 2 binaries, it moves one and then deletes another.
Please update PR description and also crate a draft PR against pytorch/pytorch repo that tests the change (similar to pytorch/pytorch#62307 ) and mention it in the test plan.

Alternatively, split this PR in two: one that replaces one URL with another (which is pretty straightforward and requires no testing) and another that deletes those extra dlls download/installation

@mszhanyi
Copy link
Contributor Author

mszhanyi commented Sep 8, 2021

@malfet , Thank your reminder. there's a test PR pytorch/pytorch#63551 for it. I planned to add the verification link when all tests passed.

At first, I planned to moved 2 binaries to S3, which the commit 3a10296 did.
When I was helping vision to enable cuda test on Windows. I found the gpu_driver_dlls.zip is only needed for cu102.

But vision has skipped all cuda tests in their CI now, so it's a little hard to me to reverify it.
Since Pytorch build is on CPU machine, I'm trying to remove gpu_driver_dlls at all to see the result

@mszhanyi
Copy link
Contributor Author

mszhanyi commented Sep 8, 2021

per malfet's suggestion. This PR only update the download URL.

It'd better to merge it soon because 4 nightly CI workflows failed on yesterday (Sep 7) due to
image

image

image

the CI should exit once the zip isn't downloaded completely.
I'll add it in another PR.

@malfet @seemethere @peterjc123

@malfet malfet merged commit 8713fe8 into pytorch:master Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace dropbox and google drive links with s3 links
4 participants