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

Re-enable vision MPS builds #8485

Merged
merged 10 commits into from
Jun 10, 2024
Merged

Conversation

huydhn
Copy link
Contributor

@huydhn huydhn commented Jun 8, 2024

Re-run the test after https://hud.pytorch.org/hud/pytorch/pytorch/75b0720a97ac5d82e8a7a1a6ae7c5f7a87d7183d is included in the next PyTorch nightly build.

cc @seemethere

Copy link

pytorch-bot bot commented Jun 8, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8485

Note: Links to docs will display an error until the docs builds have been completed.

❌ 5 New Failures

As of commit 8c6084c with merge base f1bcbd3 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@huydhn huydhn changed the title Debug vision MPS failure Re-enable vision MPS builds Jun 9, 2024
@huydhn huydhn requested a review from NicolasHug June 9, 2024 09:10
@@ -68,7 +68,7 @@ jobs:
export GPU_ARCH_TYPE=cpu
export GPU_ARCH_VERSION=''

./.github/scripts/unittest.sh
${CONDA_RUN} ./.github/scripts/unittest.sh
Copy link
Contributor Author

@huydhn huydhn Jun 9, 2024

Choose a reason for hiding this comment

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

This is not related to the issue at hand but I found it during the investigation. This is the correct way to invoke a script using pytorch/test-infra/.github/workflows/macos_job.yml@main. Otherwise, the system conda will be used instead of the one setup by macos_job and the ci env created by the script won't be clean up correctly

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @huydhn , does that mean we should use that for the rest of the jobs as well (including linux and windows)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need this on Linux because the script is run inside Docker, so cleaning things up is not an issue there and you can use your own conda installation if needed.

On the other hand, this might be needed on Windows, but I haven't seen any disk space issue there before, so maybe it's not that important there.

@huydhn huydhn marked this pull request as ready for review June 9, 2024 09:17
@huydhn huydhn mentioned this pull request Jun 9, 2024
@@ -68,7 +68,7 @@ jobs:
export GPU_ARCH_TYPE=cpu
export GPU_ARCH_VERSION=''

./.github/scripts/unittest.sh
${CONDA_RUN} ./.github/scripts/unittest.sh
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @huydhn , does that mean we should use that for the rest of the jobs as well (including linux and windows)?

@NicolasHug NicolasHug dismissed their stale review June 10, 2024 09:10

sorry - misclick

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

I can see the MPS tests being green now. Thanks a ton @huydhn for identifying the original offending PR from pytorch!

@NicolasHug NicolasHug merged commit f96c42f into pytorch:main Jun 10, 2024
68 of 73 checks passed
Copy link

Hey @NicolasHug!

You merged this PR, but no labels were added.
The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@NicolasHug NicolasHug added module: ci other if you have no clue or if you will manually handle the PR in the release notes labels Jun 10, 2024
facebook-github-bot pushed a commit that referenced this pull request Aug 2, 2024
Summary: Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

Reviewed By: vmoens

Differential Revision: D60596231

fbshipit-source-id: 8493928d6f94a3abeff6a38c9295d343127027b2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed module: ci other if you have no clue or if you will manually handle the PR in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants