Skip to content

[build] Remove FULL_CAFFE2 flag#11321

Closed
orionr wants to merge 14 commits intopytorch:masterfrom
orionr:remove-full-caffe2
Closed

[build] Remove FULL_CAFFE2 flag#11321
orionr wants to merge 14 commits intopytorch:masterfrom
orionr:remove-full-caffe2

Conversation

@orionr
Copy link
Contributor

@orionr orionr commented Sep 6, 2018

Continuing @pjh5's work to remove FULL_CAFFE2 flag completely.

With these changes you'll be able to also do something like

NO_TEST=1 python setup.py build_deps

and this will skip building tests in caffe2, aten, and c10d. By default the tests are built.

Also, BUILD_BINARIES and USE_OPENCV are OFF by default and BUILD_TEST is ON by default.

cc @mingzhe09088 @Yangqing

@orionr orionr changed the title Remove FULL_CAFFE2 flag [build] Remove FULL_CAFFE2 flag Sep 6, 2018
@ssnl
Copy link
Collaborator

ssnl commented Sep 6, 2018

What exactly will the BUILD_TEST=0 flag do? Does it turn off aten tests, torch tests, c10d tests, and c10d examples? I'm asking because I'm unable to build pytorch on devfair anymore, unless I disable all those tests/examples. FWIW, I was seeing

//usr/lib/x86_64-linux-gnu/libSM.so.6: undefined reference to `uuid_generate@UUID_1.0'

even with uuid-dev installed.

@orionr
Copy link
Contributor Author

orionr commented Sep 6, 2018

@ssnl I'll make sure the environment flag can be set and that it applies to all of those.

@orionr orionr force-pushed the remove-full-caffe2 branch 3 times, most recently from be65c40 to 3291cf2 Compare September 6, 2018 22:38
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

orionr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Collaborator

@soumith soumith left a comment

Choose a reason for hiding this comment

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

lgtm. what is "Build C++ binaries", what are the C++ binaries that we build apart from tests?

@orionr
Copy link
Contributor Author

orionr commented Sep 7, 2018

@soumith - good question. These are things like DB conversion, benchmarking, data loaders, etc.

@orionr orionr force-pushed the remove-full-caffe2 branch from 152d36a to 486067d Compare September 7, 2018 03:19
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

orionr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@orionr orionr mentioned this pull request Sep 7, 2018
@Yangqing
Copy link
Contributor

Yangqing commented Sep 7, 2018

@soumith the binaries are the native C++ binaries that C2 had, such as speed_benchmark etc. Might not be much useful in the python releases so we can probably turn it off by default.

@apaszke
Copy link
Contributor

apaszke commented Sep 7, 2018

Another downside of the binaries is that they are installed in torch/lib, but are not in .gitignore, so one needs to be careful when checking in files.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

orionr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@Yangqing
Copy link
Contributor

Yangqing commented Sep 7, 2018

non-blocking: #11388 fixes googletest issue.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

orionr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@orionr
Copy link
Contributor Author

orionr commented Sep 7, 2018

The pytorch-linux-xenial-cuda9 error looks like it's the same as one on master. Going to ignore that.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Sep 7, 2018
Summary:
Continuing pjh5's work to remove FULL_CAFFE2 flag completely.

With these changes you'll be able to also do something like

```
NO_TEST=1 python setup.py build_deps
```
and this will skip building tests in caffe2, aten, and c10d. By default the tests are built.

cc mingzhe09088 Yangqing
Pull Request resolved: pytorch/pytorch#11321

Reviewed By: mingzhe09088

Differential Revision: D9694950

Pulled By: orionr

fbshipit-source-id: ff5c4937a23d1a263378a196a5eda0cba98af0a8
@ssnl
Copy link
Collaborator

ssnl commented Sep 8, 2018

Can confirm that this fixes the build on devfair (with NO_TEST=1)! Thanks!

PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
Summary:
Continuing pjh5's work to remove FULL_CAFFE2 flag completely.

With these changes you'll be able to also do something like

```
NO_TEST=1 python setup.py build_deps
```
and this will skip building tests in caffe2, aten, and c10d. By default the tests are built.

cc mingzhe09088 Yangqing
Pull Request resolved: pytorch#11321

Reviewed By: mingzhe09088

Differential Revision: D9694950

Pulled By: orionr

fbshipit-source-id: ff5c4937a23d1a263378a196a5eda0cba98af0a8
Yangqing added a commit to Yangqing/pytorch that referenced this pull request Sep 12, 2018
facebook-github-bot pushed a commit that referenced this pull request Sep 12, 2018
Summary:
Last PR seems to have test failures, re-issuing.
Pull Request resolved: #11417

Reviewed By: orionr

Differential Revision: D9784706

Pulled By: Yangqing

fbshipit-source-id: 9e5f347e19fa2700ff69d2cd69ea7a9e01a91609
@ezyang ezyang added the merged label Jun 26, 2019
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.

7 participants