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 macos runner to self hosted #6290

Merged
merged 8 commits into from
Jul 26, 2022

Conversation

seemethere
Copy link
Member

Updates macos runner to new self hosted runners from AWS

Signed-off-by: Eli Uriegas eliuriegas@fb.com

Updates macos runner to new self hosted runners from AWS

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>
facebook-github-bot pushed a commit to pytorch/audio that referenced this pull request Jul 21, 2022
Summary:
Updates the runner to the latest apple silicon machines we have that
also run on macOS 12.4

Similar to pytorch/vision#6290

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>

Pull Request resolved: #2556

Reviewed By: atalman, mthrok

Differential Revision: D37999959

Pulled By: seemethere

fbshipit-source-id: 01d2ff01e48dcc0c4e33ed81758886fa19642aa3
@datumbox
Copy link
Contributor

We got some weird failings which are probably unrelated:

2022-07-21T11:40:40.4916820Z         t = transforms.RandomCrop(48)
2022-07-21T11:40:40.4916950Z         img = torch.ones(3, 32, 32)
2022-07-21T11:40:40.4917140Z         with pytest.raises(ValueError, match=r"Required crop size .+ is larger then input image size .+"):
2022-07-21T11:40:40.4917310Z >           t(img)
2022-07-21T11:40:40.4917610Z E           AssertionError: Regex pattern 'Required crop size .+ is larger then input image size .+' does not match 'Required crop size (48, 48) is larger than input image size (32, 32)'.

Let me update the branch with latest main and restart tests.

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

The changes LGTM. We should merge on Green CI.

@seemethere Given that now we have 45 runners for M1, we would like run these tests not after merging on main but also on the PRs. This will help us move towards beta support of M1. Given our resources are still constraint, we consider running the full unit-tests on a single python config. Any thoughts on this?

@seemethere
Copy link
Member Author

The changes LGTM. We should merge on Green CI.

@seemethere Given that now we have 45 runners for M1, we would like run these tests not after merging on main but also on the PRs. This will help us move towards beta support of M1. Given our resources are still constraint, we consider running the full unit-tests on a single python config. Any thoughts on this?

Should be fine, but would still limit it to 1 python config for the time being

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>
Signed-off-by: Eli Uriegas <eliuriegas@fb.com>
@datumbox
Copy link
Contributor

datumbox commented Jul 25, 2022

@seemethere I'm a bit confused on why this is failing. The issue seems unrelated as the changes don't affect CircleCI. Nevertheless while running the same test on main branch (see dummy PR #5009), it passes. The issue seems related to a dependency with PyTorch. Any idea?

@seemethere
Copy link
Member Author

@seemethere I'm a bit confused on why this is failing. The issue seems unrelated as the changes don't affect CircleCI. Nevertheless while running the same test on main branch (see dummy PR #5009), it passes. The issue seems related to a dependency with PyTorch. Any idea?

I think I understand the issue here, the windows runners don't have python3 installed by default so it should default to python when python3 isn't available

Signed-off-by: Eli Uriegas <eliuriegas@fb.com>
@seemethere seemethere merged commit 96f5467 into pytorch:main Jul 26, 2022
@seemethere seemethere deleted the update_macos_self_hosted branch July 26, 2022 12:00
@NicolasHug
Copy link
Member

Is this something we need to cherry-pick for the bugfix release @seemethere ?

facebook-github-bot pushed a commit that referenced this pull request Jul 28, 2022
Summary:

Reviewed By: datumbox

Differential Revision: D38154568

fbshipit-source-id: 2772603b48352bd1dec7569a2f69cf0fe4f128c1

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>
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.

4 participants