Skip to content

Add infrastructure to apply outstanding PT PRs. #183

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
Merged
merged 2 commits into from
Feb 6, 2019
Merged

Add infrastructure to apply outstanding PT PRs. #183

merged 2 commits into from
Feb 6, 2019

Conversation

dlibenzi
Copy link
Collaborator

@dlibenzi dlibenzi commented Feb 5, 2019

No description provided.

asuhan
asuhan reviewed Feb 5, 2019
View reviewed changes
Copy link
Contributor

@asuhan asuhan left a comment

Choose a reason for hiding this comment

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

Doesn't kokoro/ubuntu/common.sh need the same change?

asuhan
asuhan reviewed Feb 5, 2019
View reviewed changes
Copy link
Contributor

@asuhan asuhan left a comment

Choose a reason for hiding this comment

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

Don't we need the same changes for kokoro/ubuntu/common.sh?

.circleci/build.sh

# Install Pytorch
patch -p1 < xla/pytorch.patch
xla/scripts/apply_patches.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't kokoro/ubuntu/common.sh need the same change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah

@dlibenzi dlibenzi force-pushed the cond_patches branch 3 times, most recently from 178e9b6 to c49a8e4 Compare February 5, 2019 21:06
@asuhan asuhan requested review from ailzhang and jysohn23 February 5, 2019 21:32
ailzhang pushed a commit to ailzhang/pytorch that referenced this pull request Feb 5, 2019
changes correpsond to pytorch/xla#183
6fc6c03
@ailzhang ailzhang mentioned this pull request Feb 5, 2019
Closed
ailzhang
ailzhang approved these changes Feb 5, 2019
View reviewed changes
Copy link
Contributor

@ailzhang ailzhang left a comment

Choose a reason for hiding this comment

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

Thanks! Could you also confirm pytorch/pytorch#16781 is the only place I need to change for this to pass?
And this is good to go. :D

asuhan
asuhan suggested changes Feb 5, 2019
View reviewed changes
Copy link
Contributor

@asuhan asuhan left a comment

Choose a reason for hiding this comment

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

Since pytorch/pytorch#16585 has landed, we can remove the patch.

@dlibenzi
Copy link
Collaborator Author

dlibenzi commented Feb 5, 2019

@ailzhang yes, looks like it :)

@ailzhang
Copy link
Contributor

ailzhang commented Feb 5, 2019

@dlibenzi Given 16585 is in master now, do we still want to proceed and land this PR(after removing the unncessary patch)?

@dlibenzi
Copy link
Collaborator Author

dlibenzi commented Feb 5, 2019

PR patch removed.

@dlibenzi
Copy link
Collaborator Author

dlibenzi commented Feb 5, 2019 •
edited by asuhan
Loading

@ailzhang I think so. It cleans up the patch apply process, and covers us from future delays.

@dlibenzi
Copy link
Collaborator Author

dlibenzi commented Feb 5, 2019

@ailzhang Do I have to merge after your PR, or before?

@ailzhang
Copy link
Contributor

ailzhang commented Feb 5, 2019

@dlibenzi Please feel free to merge it once the test here is green. I will merge my PR later to get Pytorch CI passes.

@dlibenzi dlibenzi merged commit f3e974f into master Feb 6, 2019
facebook-github-bot pushed a commit to pytorch/pytorch that referenced this pull request Feb 6, 2019
@facebook-github-bot
changes to apply xla patch (#16781)
c1dff54 
Summary:
This PR will let xla tests passes after pytorch/xla#183 is in.

Will add back the branch filters once it's ready.
Pull Request resolved: #16781

Differential Revision: D13968976

Pulled By: ailzhang

fbshipit-source-id: df3b173336b3247aa56ef723569a1f68cdfa56e0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jysohn23 jysohn23 jysohn23 approved these changes

+2 more reviewers

@asuhan asuhan asuhan approved these changes

@ailzhang ailzhang ailzhang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dlibenzi @ailzhang @asuhan @jysohn23