Skip to content

Conversation

@xta0
Copy link
Contributor

@xta0 xta0 commented Nov 20, 2019

Stack from ghstack:

Summary

Recently we've found that the master branch was constantly broken due to some unwanted change being landed on mobile. The problem is that our CI was not able to detect the runtime errors.

Previous work

What's been changed in CI

  1. XCode version has been updated to 11.2.1
  2. For iOS simulator build, we'll run some unit tests( currently only one) after the build test.

Differential Revision: D18641413

@xta0 xta0 changed the title [iOS] add unit test to CI [DO NOT MERGE][iOS] add unit test to CI Nov 20, 2019
xta0 added a commit that referenced this pull request Nov 20, 2019
ghstack-source-id: 3bb17a0
Pull Request resolved: #30133
xta0 added a commit that referenced this pull request Nov 20, 2019
ghstack-source-id: 563a0d8
Pull Request resolved: #30133
xta0 added a commit that referenced this pull request Nov 20, 2019
ghstack-source-id: d902bbe
Pull Request resolved: #30133
xta0 added a commit that referenced this pull request Nov 20, 2019
ghstack-source-id: 45269d2
Pull Request resolved: #30133
xta0 added a commit that referenced this pull request Nov 20, 2019
ghstack-source-id: b128234
Pull Request resolved: #30133
@xta0 xta0 changed the title [DO NOT MERGE][iOS] add unit test to CI [DO NOT MERGE][iOS] add unit test to CI workflow Nov 20, 2019
xta0 added a commit that referenced this pull request Nov 21, 2019
ghstack-source-id: d137caa
Pull Request resolved: #30133
xta0 added a commit that referenced this pull request Nov 21, 2019
ghstack-source-id: b47d9dc
Pull Request resolved: #30133
@xta0 xta0 requested review from kostmo and shoumikhin November 21, 2019 04:30
@xta0 xta0 changed the title [DO NOT MERGE][iOS] add unit test to CI workflow [iOS] add unit test to CI jobs Nov 21, 2019
@xta0 xta0 requested review from ezyang and ljk53 November 21, 2019 04:31
PROJ_ROOT=/Users/distiller/project
source ~/anaconda/bin/activate
#install the latest version of PyTorch and TorchVision
#TODO: How to install the nightly build?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ezyang is it possible to install a PyTorch nightly build on MacOS via pip? I searched a lil bit, didn't find the right command to do it. The goal here is to use a recent (up-to-date) version of PyTorch to generate a torchscript model for the iOS TestApp to test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like:

pip install --pre torch torchvision -f https://download.pytorch.org/whl/nightly/cpu/torch_nightly.html

as suggested in https://pytorch.org/get-started/locally/

@xta0 xta0 changed the title [iOS] add unit test to CI jobs [iOS] add unit tests to iOS CI jobs Nov 21, 2019
- should_run_job
- checkout
- run_brew_for_ios_build
- run_brew_for_ios_build
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove white space?

_module.eval();
std::vector<c10::IValue> inputs;
inputs.push_back(torch::ones({1, 3, 224, 224}, at::ScalarType::Float));
torch::autograd::AutoGradMode guard(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the old RAII use didn't do anything :P

@ezyang
Copy link
Contributor

ezyang commented Nov 21, 2019

I'm a little concerned about relying on nightlies for per PR tests, because it means that a botched nightly build will break all signal on this job, and you won't have any easy remediation. In this sense, it is almost better to use the official build, which will rev much less frequently. However, the official build is only really OK if we maintain backwards-compatibility (which we should be, right @houseroad ?) So I feel the patch as is is better.

### Summary

Recently we've found that the master branch was constantly broken due to some unwanted change being landed on mobile. The problem is that our CI was not able to detect the runtime errors. 

### Previous work

- Add an unit test target to the iOS TestApp ( #29962 )
- Update Fastlane to run tests ( #29963 )

### What's been changed in CI

1. XCode version has been updated to 11.2.1 
2. For iOS simulator build, we'll run some unit tests( currently only one) after the build test.

Differential Revision: [D18641413](https://our.internmc.facebook.com/intern/diff/D18641413)
### Summary

Recently we've found that the master branch was constantly broken due to some unwanted change being landed on mobile. The problem is that our CI was not able to detect the runtime errors. 

### Previous work

- Add an unit test target to the iOS TestApp ( #29962 )
- Update Fastlane to run tests ( #29963 )

### What's been changed in CI

1. XCode version has been updated to 11.2.1 
2. For iOS simulator build, we'll run some unit tests( currently only one) after the build test.

Differential Revision: [D18641413](https://our.internmc.facebook.com/intern/diff/D18641413)
xta0 added a commit that referenced this pull request Nov 22, 2019
ghstack-source-id: 15233f9
Pull Request resolved: #30133
### Summary

Recently we've found that the master branch was constantly broken due to some unwanted change being landed on mobile. The problem is that our CI was not able to detect the runtime errors. 

### Previous work

- Add an unit test target to the iOS TestApp ( #29962 )
- Update Fastlane to run tests ( #29963 )

### What's been changed in CI

1. XCode version has been updated to 11.2.1 
2. For iOS simulator build, we'll run some unit tests( currently only one) after the build test.

Differential Revision: [D18641413](https://our.internmc.facebook.com/intern/diff/D18641413)
xta0 added a commit that referenced this pull request Nov 22, 2019
ghstack-source-id: a433e02
Pull Request resolved: #30133
### Summary

Recently we've found that the master branch was constantly broken due to some unwanted change being landed on mobile. The problem is that our CI was not able to detect the runtime errors. 

### Previous work

- Add an unit test target to the iOS TestApp ( #29962 )
- Update Fastlane to run tests ( #29963 )

### What's been changed in CI

1. XCode version has been updated to 11.2.1 
2. For iOS simulator build, we'll run some unit tests( currently only one) after the build test.

Differential Revision: [D18641413](https://our.internmc.facebook.com/intern/diff/D18641413)
xta0 added a commit that referenced this pull request Nov 22, 2019
ghstack-source-id: 56d9113
Pull Request resolved: #30133
@hlu1
Copy link
Contributor

hlu1 commented Nov 22, 2019

@xta0, I'm reverting this PR because it broke master. I saw that the fix #30327 is on the way. Please make sure it doesn't break other jobs.

@xta0 xta0 mentioned this pull request Nov 26, 2019
@facebook-github-bot facebook-github-bot deleted the gh/xta0/44/head branch December 10, 2019 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants