Skip to content

[build] Remove BUILD_CAFFE2 and build everything#8338

Closed
orionr wants to merge 30 commits intopytorch:masterfrom
orionr:remove-use-caffe2-and-aten
Closed

[build] Remove BUILD_CAFFE2 and build everything#8338
orionr wants to merge 30 commits intopytorch:masterfrom
orionr:remove-use-caffe2-and-aten

Conversation

@orionr
Copy link
Contributor

@orionr orionr commented Jun 11, 2018

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

@orionr orionr changed the title [WIP] Completely remove USE_CAFFE2 and USE_ATEN [WIP] Completely remove BUILD_CAFFE2 and BUILD_ATEN Jun 11, 2018
CMakeLists.txt Outdated

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@ezyang
Copy link
Contributor

ezyang commented Jun 12, 2018

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.

@Yangqing
Copy link
Contributor

@ezyang most of the changes are basically removing the if(BUILD_ATEN) or if(BUILD_CAFFE2) blocks, hence lots of indentation.

@orionr orionr force-pushed the remove-use-caffe2-and-aten branch from 945f7a0 to d968f6c Compare June 13, 2018 18:14
@orionr
Copy link
Contributor Author

orionr commented Jun 14, 2018

@ezyang, when we've fixed all the build failures and done more than just indenting / if blocks I'll update in the description.

@orionr orionr force-pushed the remove-use-caffe2-and-aten branch 4 times, most recently from c3d3163 to 31ef861 Compare June 18, 2018 18:28
@mingzhe09088 mingzhe09088 force-pushed the remove-use-caffe2-and-aten branch from 8b8d6ce to 7ee09ad Compare June 22, 2018 16:53
@ezyang
Copy link
Contributor

ezyang commented Jun 27, 2018

@pytorchbot retest this please

@mingzhe09088 mingzhe09088 force-pushed the remove-use-caffe2-and-aten branch 2 times, most recently from fd63c2c to ff4a418 Compare July 3, 2018 16:07
@anderspapitto
Copy link
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 -w on the command line

@ezyang
Copy link
Contributor

ezyang commented Jul 9, 2018

@pytorchbot retest this please

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.

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.

10 participants