[build] Remove BUILD_CAFFE2 and build everything #8338
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orionr
requested review from
apaszke,
colesbury,
ezyang,
gchanan,
soumith and
zdevito
as code owners
orionr
changed the title
ezyang
reviewed
Jun 12, 2018
CMakeLists.txt
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Contributor
|
There's a lot of reindenting going on in this diff, which makes it hard to tell what the changes are. With my code reviewer hat on, it's super helpful if there is a changelog enumerating what the substantive changes to the cmake are. It helps a lot when doing review, since I know what to be looking for. |
Contributor
|
@ezyang most of the changes are basically removing the if(BUILD_ATEN) or if(BUILD_CAFFE2) blocks, hence lots of indentation. |
orionr
force-pushed
the
remove-use-caffe2-and-aten
branch
from
945f7a0 to
d968f6c
Compare
Contributor
Author
|
@ezyang, when we've fixed all the build failures and done more than just indenting / |
orionr
force-pushed
the
remove-use-caffe2-and-aten
branch
4 times, most recently
from
c3d3163 to
31ef861
Compare
mingzhe09088
force-pushed
the
remove-use-caffe2-and-aten
branch
from
8b8d6ce to
7ee09ad
Compare
Contributor
|
@pytorchbot retest this please |
mingzhe09088
force-pushed
the
remove-use-caffe2-and-aten
branch
2 times, most recently
from
fd63c2c to
ff4a418
Compare
Contributor
|
@ezyang "There's a lot of reindenting going on in this diff, which makes it hard to tell what the changes are." - take a look at the "Hide whitespace changes" checkbox under "Diff settings" at the top of the diff. It's analogous to |
This was referenced Jul 9, 2018
Closed
Contributor
|
@pytorchbot retest this please |
This was referenced Jul 9, 2018
Closed
mingzhe09088
force-pushed
the
remove-use-caffe2-and-aten
branch
from
5419eb1 to
1c11f4c
Compare
Contributor
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
orionr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
mingzhe09088
approved these changes
Aug 31, 2018
zdevito
pushed a commit
to zdevito/ATen
that referenced
this pull request
Aug 31, 2018
Summary: This completely removes BUILD_CAFFE2 from CMake. There is still a little bit of "full build" stuff in setup.py that enables USE_CUDNN and BUILD_PYTHON, but otherwise everything should be enabled for PyTorch as well as Caffe2. This gets us a lot closer to full unification. cc mingzhe09088, pjh5, ezyang, smessmer, Yangqing Pull Request resolved: pytorch/pytorch#8338 Reviewed By: mingzhe09088 Differential Revision: D9600513 Pulled By: orionr fbshipit-source-id: 9f6ca49df35b920d3439dcec56e7b26ad4768b7d
ezyang
added a commit
to ezyang/pytorch
that referenced
this pull request
Sep 1, 2018
This reverts commit 6508db7.
PenghuiCheng
pushed a commit
to PenghuiCheng/pytorch
that referenced
this pull request
Sep 11, 2018
Summary: Breaking out of pytorch#8338 to completely remove build_aten and use_aten. Pull Request resolved: pytorch#10469 Reviewed By: orionr Differential Revision: D9413639 Pulled By: mingzhe09088 fbshipit-source-id: b7203aa4f5f2bb95c504c8dc187a3167f2570183
PenghuiCheng
pushed a commit
to PenghuiCheng/pytorch
that referenced
this pull request
Sep 11, 2018
Summary: Breaking out of pytorch#8338 cc mingzhe09088 Yangqing Pull Request resolved: pytorch#11000 Differential Revision: D9557474 Pulled By: orionr fbshipit-source-id: 7d84914b67ff37bdb7738f9b7846dfeb5b975c00
PenghuiCheng
pushed a commit
to PenghuiCheng/pytorch
that referenced
this pull request
Sep 11, 2018
Summary: Breaking out of pytorch#8338 This test fails once we start building with `-DUSE_GLOG=OFF` since the non-glog logging case doesn't support flushing or streaming to the right location. For now, we just disable this test in that case. cc Yangqing mingzhe09088 Pull Request resolved: pytorch#10999 Reviewed By: mingzhe09088 Differential Revision: D9557488 Pulled By: orionr fbshipit-source-id: 8b306f210411dfc8ccc404bdccf77ddcd36a4830
PenghuiCheng
pushed a commit
to PenghuiCheng/pytorch
that referenced
this pull request
Sep 11, 2018
Summary: Breaking this out of pytorch#8338 mingzhe09088's fix of the docstrings for Windows builds. Unfortunately some versions of Windows seem to try and parse the `#` inside the string as a pre-processor declaration. We might need to change this to something else later, but want to get this landed first. cc mingzhe09088 Yangqing Pull Request resolved: pytorch#10998 Reviewed By: mingzhe09088 Differential Revision: D9557480 Pulled By: orionr fbshipit-source-id: c6a6237c27b7cf35c81133fd9faefead675a9f59
PenghuiCheng
pushed a commit
to PenghuiCheng/pytorch
that referenced
this pull request
Sep 11, 2018
Summary: Extracted from pytorch#8338 cc mingzhe09088 Pull Request resolved: pytorch#11034 Reviewed By: mingzhe09088 Differential Revision: D9582801 Pulled By: orionr fbshipit-source-id: b41ca1bebf6cf62fff2a2b8caf4c94af3e43db00
PenghuiCheng
pushed a commit
to PenghuiCheng/pytorch
that referenced
this pull request
Sep 11, 2018
Summary: This completely removes BUILD_CAFFE2 from CMake. There is still a little bit of "full build" stuff in setup.py that enables USE_CUDNN and BUILD_PYTHON, but otherwise everything should be enabled for PyTorch as well as Caffe2. This gets us a lot closer to full unification. cc mingzhe09088, pjh5, ezyang, smessmer, Yangqing Pull Request resolved: pytorch#8338 Reviewed By: mingzhe09088 Differential Revision: D9600513 Pulled By: orionr fbshipit-source-id: 9f6ca49df35b920d3439dcec56e7b26ad4768b7d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This completely removes BUILD_CAFFE2 from CMake. There is still a little bit of "full build" stuff in setup.py that enables USE_CUDNN and BUILD_PYTHON, but otherwise everything should be enabled for PyTorch as well as Caffe2. This gets us a lot closer to full unification.
cc @mingzhe09088, @pjh5, @ezyang, @smessmer, @Yangqing